From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vaow3-0004Jd-BJ for qemu-devel@nongnu.org; Mon, 28 Oct 2013 11:41:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vaovx-0006cb-9x for qemu-devel@nongnu.org; Mon, 28 Oct 2013 11:40:55 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:37021 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vaovw-0006cR-KI for qemu-devel@nongnu.org; Mon, 28 Oct 2013 11:40:49 -0400 Date: Mon, 28 Oct 2013 16:40:39 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20131028154038.GG2890@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] How to introduce bs->node_name ? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, stefanha@redhat.com Hi list, After a discussion on irc we have two potential solution in order to intr= oduce a new bs->node_name member in order to be able to manipulate the graph fr= om the monitors. The first one is to make the QMP device parameter of the block commands o= ptional and add the node-name parameter as a second optional parameter. This is Markus prefered solution and Eric is ok with making mandatory par= ameters optional in QMP. The second one suggested by Kevin Would be to add some magic to the new node_name member by making it equal to device_name for backends and then = making the qmp commands operate only on node-names. My personnal suggestion would be that non specified node-name would be se= t to "undefined" meaning that no operation could occur on this bs. For QMP access the device_name is accessed via bdrv_find() in a few place= in blockdev. Here are the occurences of it: commit ------ void do_commit(Monitor *mon, const QDict *qdict) { const char *device =3D qdict_get_str(qdict, "device"); BlockDriverState *bs; int ret; if (!strcmp(device, "all")) { ret =3D bdrv_commit_all(); } else { bs =3D bdrv_find(device); if (!bs) { monitor_printf(mon, "Device '%s' not found\n", device); return; } ret =3D bdrv_commit(bs); } if (ret < 0) { monitor_printf(mon, "'commit' error for '%s': %s\n", device, strerror(-ret)); } } internal snapshot deletion -------------------------- SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *devi= ce, bool has_id, const char *id, bool has_name, const char *name= , Error **errp) { BlockDriverState *bs =3D bdrv_find(device); QEMUSnapshotInfo sn; Error *local_err =3D NULL; SnapshotInfo *info =3D NULL; Internal snapshot preparation ----------------------------- static void internal_snapshot_prepare(BlkTransactionState *common, Error **errp) { const char *device; const char *name; BlockDriverState *bs; QEMUSnapshotInfo old_sn, *sn; bool ret; qemu_timeval tv; BlockdevSnapshotInternal *internal; InternalSnapshotState *state; int ret1; g_assert(common->action->kind =3D=3D TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC); internal =3D common->action->blockdev_snapshot_internal_sync; state =3D DO_UPCAST(InternalSnapshotState, common, common); /* 1. parse input */ device =3D internal->device; name =3D internal->name; /* 2. check for validation */ bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } Drive backup ------------ static void drive_backup_prepare(BlkTransactionState *common, Error **err= p) { DriveBackupState *state =3D DO_UPCAST(DriveBackupState, common, commo= n); DriveBackup *backup; Error *local_err =3D NULL; assert(common->action->kind =3D=3D TRANSACTION_ACTION_KIND_DRIVE_BACK= UP); backup =3D common->action->drive_backup; qmp_drive_backup(backup->device, backup->target, backup->has_format, backup->format, backup->sync, backup->has_mode, backup->mode, backup->has_speed, backup->speed, backup->has_on_source_error, backup->on_source_error= , backup->has_on_target_error, backup->on_target_error= , &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); state->bs =3D NULL; state->job =3D NULL; return; } state->bs =3D bdrv_find(backup->device); state->job =3D state->bs->job; } Eject which should operate on backends -------------------------------------- void qmp_eject(const char *device, bool has_force, bool force, Error **er= rp) { BlockDriverState *bs; bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } eject_device(bs, force, errp); } QCow2 crypto ------------ void qmp_block_passwd(const char *device, const char *password, Error **e= rrp) { BlockDriverState *bs; int err; bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } err =3D bdrv_set_key(bs, password); if (err =3D=3D -EINVAL) { error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(b= s)); return; } else if (err < 0) { error_set(errp, QERR_INVALID_PASSWORD); return; } } Change blockdev (I don't know what it is used for) -------------------------------------------------- void qmp_change_blockdev(const char *device, const char *filename, bool has_format, const char *format, Error **err= p) { BlockDriverState *bs; BlockDriver *drv =3D NULL; int bdrv_flags; Error *err =3D NULL; bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } if (format) { drv =3D bdrv_find_whitelisted_format(format, bs->read_only); if (!drv) { error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); return; } } eject_device(bs, 0, &err); if (error_is_set(&err)) { error_propagate(errp, err); return; } bdrv_flags =3D bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; bdrv_flags |=3D bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp); } Throttling ---------- void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t b= ps_rd, int64_t bps_wr, int64_t iops, int64_t iops_rd, int64_t iops_wr, bool has_bps_max, int64_t bps_max, bool has_bps_rd_max, int64_t bps_rd_max, bool has_bps_wr_max, int64_t bps_wr_max, bool has_iops_max, int64_t iops_max, bool has_iops_rd_max, int64_t iops_rd_max, bool has_iops_wr_max, int64_t iops_wr_max, bool has_iops_size, int64_t iops_size, Error **errp) { ThrottleConfig cfg; BlockDriverState *bs; bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } Drive deletion -------------- int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) { const char *id =3D qdict_get_str(qdict, "id"); BlockDriverState *bs; bs =3D bdrv_find(id); if (!bs) { qerror_report(QERR_DEVICE_NOT_FOUND, id); return -1; } block resize ------------ void qmp_block_resize(const char *device, int64_t size, Error **errp) { BlockDriverState *bs; int ret; bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } streaming --------- void qmp_block_stream(const char *device, bool has_base, const char *base, bool has_speed, int64_t speed, bool has_on_error, BlockdevOnError on_error, Error **errp) { BlockDriverState *bs; BlockDriverState *base_bs =3D NULL; Error *local_err =3D NULL; if (!has_on_error) { on_error =3D BLOCKDEV_ON_ERROR_REPORT; } bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } commit ------ void qmp_block_commit(const char *device, bool has_base, const char *base, const char *top, bool has_speed, int64_t speed, Error **errp) { BlockDriverState *bs; BlockDriverState *base_bs, *top_bs; Error *local_err =3D NULL; /* This will be part of the QMP command, if/when the * BlockdevOnError change for blkmirror makes it in */ BlockdevOnError on_error =3D BLOCKDEV_ON_ERROR_REPORT; /* drain all i/o before commits */ bdrv_drain_all(); bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } backup: ------- void qmp_drive_backup(const char *device, const char *target, bool has_format, const char *format, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, bool has_on_source_error, BlockdevOnError on_source= _error, bool has_on_target_error, BlockdevOnError on_target= _error, Error **errp) { BlockDriverState *bs; BlockDriverState *target_bs; BlockDriverState *source =3D NULL; BlockDriver *drv =3D NULL; Error *local_err =3D NULL; int flags; int64_t size; int ret; if (!has_speed) { speed =3D 0; } if (!has_on_source_error) { on_source_error =3D BLOCKDEV_ON_ERROR_REPORT; } if (!has_on_target_error) { on_target_error =3D BLOCKDEV_ON_ERROR_REPORT; } if (!has_mode) { mode =3D NEW_IMAGE_MODE_ABSOLUTE_PATHS; } bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } mirror ------ void qmp_drive_mirror(const char *device, const char *target, bool has_format, const char *format, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, bool has_granularity, uint32_t granularity, bool has_buf_size, int64_t buf_size, bool has_on_source_error, BlockdevOnError on_source= _error, bool has_on_target_error, BlockdevOnError on_target= _error, Error **errp) { BlockDriverState *bs; BlockDriverState *source, *target_bs; BlockDriver *drv =3D NULL; Error *local_err =3D NULL; int flags; int64_t size; int ret; if (!has_speed) { speed =3D 0; } if (!has_on_source_error) { on_source_error =3D BLOCKDEV_ON_ERROR_REPORT; } if (!has_on_target_error) { on_target_error =3D BLOCKDEV_ON_ERROR_REPORT; } if (!has_mode) { mode =3D NEW_IMAGE_MODE_ABSOLUTE_PATHS; } if (!has_granularity) { granularity =3D 0; } if (!has_buf_size) { buf_size =3D DEFAULT_MIRROR_BUF_SIZE; } if (granularity !=3D 0 && (granularity < 512 || granularity > 1048576= * 64)) { error_set(errp, QERR_INVALID_PARAMETER, device); return; } if (granularity & (granularity - 1)) { error_set(errp, QERR_INVALID_PARAMETER, device); return; } bs =3D bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } find block job -------------- static BlockJob *find_block_job(const char *device) { BlockDriverState *bs; bs =3D bdrv_find(device); if (!bs || !bs->job) { return NULL; } return bs->job; } Very few of them operate on what is destined to become block backend and = most of them should be able to operate on nodes of the graph; What solution do you prefer ? Best regards Beno=EEt