Re: [Qemu-devel] How to introduce bs-node_name ?
Le Monday 04 Nov 2013 à 19:33:21 (+0800), Fam Zheng a écrit : On 11/04/2013 07:06 PM, Kevin Wolf wrote: Am 04.11.2013 um 10:48 hat Fam Zheng geschrieben: On 11/04/2013 05:31 PM, Stefan Hajnoczi wrote: On Wed, Oct 30, 2013 at 02:49:32PM +0100, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). This proposal gets my vote. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. Different legacy commands given the same device name might need to operate on different nodes. Could you give an example for this? Dynamic renaming does not solve this problem, so I'm not convinced we can always choose a device name matching a node name. Device name commands are higher-level than graph node commands. For example, block_set_io_throttle makes sense on a device but less sense on a graph node, unless we add the implicit assumption that the new throttling node is created on top of the given node or updated in place if the throttling node already exists (!!). Throttling a node could be useful too, for example if we want to throttle backing_hd which is on shared storage, but not to throttle on the local image. My ignorant question is: Why can't we just use one namespace, make sure no name collision between node_name and device_name, or even just drop device_name, so we treat the root node's node_name as device_name? For commands that only accept a device, this can be enforced in its implementation by checking against the whole graph to verify this. Markus described it somewhere in this thread: Live snapshots. Currently, the device_name moves to the new BDS on the top (and compatibility requires us to keep it that way), whereas a node name should, of course, stay at its node. When you consider this, the single namespace, as much as I would have loved it, is pretty much dead. Good everyone agree on the direction to take. I'll write some code. Best regards Benoît Thanks for explaining (again). I get the reason now. Fam
Re: [Qemu-devel] How to introduce bs-node_name ?
On Wed, Oct 30, 2013 at 02:49:32PM +0100, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). This proposal gets my vote. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. Different legacy commands given the same device name might need to operate on different nodes. Dynamic renaming does not solve this problem, so I'm not convinced we can always choose a device name matching a node name. Device name commands are higher-level than graph node commands. For example, block_set_io_throttle makes sense on a device but less sense on a graph node, unless we add the implicit assumption that the new throttling node is created on top of the given node or updated in place if the throttling node already exists (!!). Stefan
Re: [Qemu-devel] How to introduce bs-node_name ?
On 11/04/2013 05:31 PM, Stefan Hajnoczi wrote: On Wed, Oct 30, 2013 at 02:49:32PM +0100, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). This proposal gets my vote. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. Different legacy commands given the same device name might need to operate on different nodes. Could you give an example for this? Dynamic renaming does not solve this problem, so I'm not convinced we can always choose a device name matching a node name. Device name commands are higher-level than graph node commands. For example, block_set_io_throttle makes sense on a device but less sense on a graph node, unless we add the implicit assumption that the new throttling node is created on top of the given node or updated in place if the throttling node already exists (!!). Throttling a node could be useful too, for example if we want to throttle backing_hd which is on shared storage, but not to throttle on the local image. My ignorant question is: Why can't we just use one namespace, make sure no name collision between node_name and device_name, or even just drop device_name, so we treat the root node's node_name as device_name? For commands that only accept a device, this can be enforced in its implementation by checking against the whole graph to verify this. Fam
Re: [Qemu-devel] How to introduce bs-node_name ?
Am 04.11.2013 um 10:48 hat Fam Zheng geschrieben: On 11/04/2013 05:31 PM, Stefan Hajnoczi wrote: On Wed, Oct 30, 2013 at 02:49:32PM +0100, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). This proposal gets my vote. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. Different legacy commands given the same device name might need to operate on different nodes. Could you give an example for this? Dynamic renaming does not solve this problem, so I'm not convinced we can always choose a device name matching a node name. Device name commands are higher-level than graph node commands. For example, block_set_io_throttle makes sense on a device but less sense on a graph node, unless we add the implicit assumption that the new throttling node is created on top of the given node or updated in place if the throttling node already exists (!!). Throttling a node could be useful too, for example if we want to throttle backing_hd which is on shared storage, but not to throttle on the local image. My ignorant question is: Why can't we just use one namespace, make sure no name collision between node_name and device_name, or even just drop device_name, so we treat the root node's node_name as device_name? For commands that only accept a device, this can be enforced in its implementation by checking against the whole graph to verify this. Markus described it somewhere in this thread: Live snapshots. Currently, the device_name moves to the new BDS on the top (and compatibility requires us to keep it that way), whereas a node name should, of course, stay at its node. When you consider this, the single namespace, as much as I would have loved it, is pretty much dead. Kevin
Re: [Qemu-devel] How to introduce bs-node_name ?
Am 01.11.2013 um 16:12 hat Luiz Capitulino geschrieben: On Fri, 01 Nov 2013 08:59:20 -0600 Eric Blake ebl...@redhat.com wrote: On 11/01/2013 08:51 AM, Luiz Capitulino wrote: On Wed, 30 Oct 2013 13:25:35 -0600 Eric Blake ebl...@redhat.com wrote: On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. Does this proposal makes device optional for existing commands? If it does then I'm afraid it breaks compatibility because if you don't specify a device you'll get an error today. Changing from error to success is not backwards-incompatible. Old applications will ALWAYS supply device (because it used to be mandatory). That is, a management application that was intentionally omitting 'device' and expecting an error is so unlikely to exist that we can consider such a management app as buggy. Doing such changes makes me nervous nevertheless. In my mind a stable API doesn't change. Of course that there might exceptions, but 99.9% of those exceptions should be bug fixes not deliberate API extensions. Stable API means that whatever was working before keeps working. Nothing less, but also nothing more. If an extension that changes from error to success is breaking the API, adding a new QMP command is breaking the API as well. Because sending an unknown command means returning an error... A more compelling argument against it is the quality of the resulting command. I'm sure it's going to be anything but a simple, clean API. I consider one command with an argument made optional a higher quality API than having two different commands that do almost the same, except that the older one has a mandatory argument where the newer one has an optional argument. Kevin
Re: [Qemu-devel] How to introduce bs-node_name ?
On 11/04/2013 07:06 PM, Kevin Wolf wrote: Am 04.11.2013 um 10:48 hat Fam Zheng geschrieben: On 11/04/2013 05:31 PM, Stefan Hajnoczi wrote: On Wed, Oct 30, 2013 at 02:49:32PM +0100, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). This proposal gets my vote. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. Different legacy commands given the same device name might need to operate on different nodes. Could you give an example for this? Dynamic renaming does not solve this problem, so I'm not convinced we can always choose a device name matching a node name. Device name commands are higher-level than graph node commands. For example, block_set_io_throttle makes sense on a device but less sense on a graph node, unless we add the implicit assumption that the new throttling node is created on top of the given node or updated in place if the throttling node already exists (!!). Throttling a node could be useful too, for example if we want to throttle backing_hd which is on shared storage, but not to throttle on the local image. My ignorant question is: Why can't we just use one namespace, make sure no name collision between node_name and device_name, or even just drop device_name, so we treat the root node's node_name as device_name? For commands that only accept a device, this can be enforced in its implementation by checking against the whole graph to verify this. Markus described it somewhere in this thread: Live snapshots. Currently, the device_name moves to the new BDS on the top (and compatibility requires us to keep it that way), whereas a node name should, of course, stay at its node. When you consider this, the single namespace, as much as I would have loved it, is pretty much dead. Thanks for explaining (again). I get the reason now. Fam
Re: [Qemu-devel] How to introduce bs-node_name ?
On Mon, 4 Nov 2013 12:13:59 +0100 Kevin Wolf kw...@redhat.com wrote: Am 01.11.2013 um 16:12 hat Luiz Capitulino geschrieben: On Fri, 01 Nov 2013 08:59:20 -0600 Eric Blake ebl...@redhat.com wrote: On 11/01/2013 08:51 AM, Luiz Capitulino wrote: On Wed, 30 Oct 2013 13:25:35 -0600 Eric Blake ebl...@redhat.com wrote: On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. Does this proposal makes device optional for existing commands? If it does then I'm afraid it breaks compatibility because if you don't specify a device you'll get an error today. Changing from error to success is not backwards-incompatible. Old applications will ALWAYS supply device (because it used to be mandatory). That is, a management application that was intentionally omitting 'device' and expecting an error is so unlikely to exist that we can consider such a management app as buggy. Doing such changes makes me nervous nevertheless. In my mind a stable API doesn't change. Of course that there might exceptions, but 99.9% of those exceptions should be bug fixes not deliberate API extensions. Stable API means that whatever was working before keeps working. Nothing less, but also nothing more. If an extension that changes from error to success is breaking the API, adding a new QMP command is breaking the API as well. Because sending an unknown command means returning an error... Let's not turn this into a surreal discussion, I'm sure we all want the best for our users. There's another problem, btw. We're not doing command extensions for input arguments before heaving a way to discover them. Even if there's consensus that extending the command is okay, we need the query-qmp-schema command merged first. IMO, this a blocker requirement (although it shouldn't be difficult to finalize what has been posted already). A more compelling argument against it is the quality of the resulting command. I'm sure it's going to be anything but a simple, clean API. I consider one command with an argument made optional a higher quality API than having two different commands that do almost the same, except that the older one has a mandatory argument where the newer one has an optional argument. I wouldn't expect the new command to be just a duplication of the old one with a new argument. I'm expecting it to have a one-cover all argument or to have a dict or something that makes more sense for the new capabilities. But yes, we need a schema example to see how it would look like.
Re: [Qemu-devel] How to introduce bs-node_name ?
On Wed, 30 Oct 2013 13:25:35 -0600 Eric Blake ebl...@redhat.com wrote: On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. Does this proposal makes device optional for existing commands? If it does then I'm afraid it breaks compatibility because if you don't specify a device you'll get an error today. Have you considered adding new commands instead? I think we should review with the QMP schema first, code second. Yes, get the interface right, and then it's easier to review the code that implements the interface. Agreed.
Re: [Qemu-devel] How to introduce bs-node_name ?
On 11/01/2013 08:51 AM, Luiz Capitulino wrote: On Wed, 30 Oct 2013 13:25:35 -0600 Eric Blake ebl...@redhat.com wrote: On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. Does this proposal makes device optional for existing commands? If it does then I'm afraid it breaks compatibility because if you don't specify a device you'll get an error today. Changing from error to success is not backwards-incompatible. Old applications will ALWAYS supply device (because it used to be mandatory). That is, a management application that was intentionally omitting 'device' and expecting an error is so unlikely to exist that we can consider such a management app as buggy. For more examples of conversion from error to success, consider the 'block-commit' command. As introduced in 1.3, we did not yet have the implementation to commit the current image. But we designed the command with a view to the future (which we are nearly at, by the way, although I don't know if it will make 1.7 or be delayed to 1.8). In fact, we specifically made the 'top' argument mandatory at the time, and documented that if 'top' was the active layer that the command would fail; but with the full intent of removing the error and instead succeeding once we implement full commit; we also discussed the possibility in the 1.3 time-frame that 'top' could be made optional once block-commit could manage the current image. Have you considered adding new commands instead? I'm still not convinced we need new commands yet. But again, proposing the QMP schema first will make that clearer. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] How to introduce bs-node_name ?
On Fri, 01 Nov 2013 08:59:20 -0600 Eric Blake ebl...@redhat.com wrote: On 11/01/2013 08:51 AM, Luiz Capitulino wrote: On Wed, 30 Oct 2013 13:25:35 -0600 Eric Blake ebl...@redhat.com wrote: On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. Does this proposal makes device optional for existing commands? If it does then I'm afraid it breaks compatibility because if you don't specify a device you'll get an error today. Changing from error to success is not backwards-incompatible. Old applications will ALWAYS supply device (because it used to be mandatory). That is, a management application that was intentionally omitting 'device' and expecting an error is so unlikely to exist that we can consider such a management app as buggy. Doing such changes makes me nervous nevertheless. In my mind a stable API doesn't change. Of course that there might exceptions, but 99.9% of those exceptions should be bug fixes not deliberate API extensions. A more compelling argument against it is the quality of the resulting command. I'm sure it's going to be anything but a simple, clean API. Anyways, let's wait for a concrete proposal to have more concrete feedback.
Re: [Qemu-devel] How to introduce bs-node_name ?
[Note cc: Eric Luiz, because this also relates to QMP] Benoît Canet benoit.ca...@irqsave.net writes: Hi list, After a discussion on irc we have two potential solution in order to introduce a new bs-node_name member in order to be able to manipulate the graph from the monitors. The first one is to make the QMP device parameter of the block commands optional and add the node-name parameter as a second optional parameter. This is Markus prefered solution and Eric is ok with making mandatory parameters 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. Needs elaboration. Let me try. Currently, a BlockDriverState (short: BDS) has a device_name only when it's a root of a BDS tree. These roots are the drives defined with -drive friends. There's code relying on device_name implies root, and vice versa. We're working on giving the user much more control over how block drivers are combined into backends. One aspect of that is we need a way for users to refer to a BDS. Elsewhere in QEMU, we let users tack IDs to stuff they create. The new node_name suggested by Benoît is exactly that. Another aspect is that we're going to create a BlockBackend (short: BB) object that captures the stuff now in BDS that is really about the whole backend (and thus is used only in root BDSes now). Semantically, device_name belongs to the whole backend, so it should move into BB. So far, QMP commands identify the object to be worked on by its device_name, typically as parameter device. Consequently, they can only work on roots now. This is just fine for QMP commands that want to work on a backend. It's not fine for QMP commands that want to work on an arbitrary, possibly non-root BDS. The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. What about situations where the old code rejects a value of device? The new code may resolve it to a non-root BDS that happens to have that ID... What about dynamic reconfiguration changing the root? Example: a synchronous snapshot splices in a QCOW2, which becomes the new root. In the current code, device_name refers to the new root. Wouldn't that require the BDS ID of the old root moves to the new root? But that would mean BDS IDs change! To be honest, this scares me. I've been burned by convenience magic and compatibility magic often enough already. My personnal suggestion would be that non specified node-name would be set to undefined meaning that no operation could occur on this bs. Yes, that's how IDs work elsewhere. For QMP access the device_name is accessed via bdrv_find() in a few place in blockdev. Here are the occurences of it: [QMP and HMP code using bdrv_find() snipped] I think we should review with the QMP schema first, code second. 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 ? Should be obvious by now :)
Re: [Qemu-devel] How to introduce bs-node_name ?
On 10/30/2013 07:49 AM, Markus Armbruster wrote: The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). I like this one as well. The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. What about situations where the old code rejects a value of device? The new code may resolve it to a non-root BDS that happens to have that ID... What about dynamic reconfiguration changing the root? Example: a synchronous snapshot splices in a QCOW2, which becomes the new root. In the current code, device_name refers to the new root. Wouldn't that require the BDS ID of the old root moves to the new root? But that would mean BDS IDs change! Having device_name tied to the BB, and ID tied to each BDS, seems much more workable in the long run. My personnal suggestion would be that non specified node-name would be set to undefined meaning that no operation could occur on this bs. Yes, that's how IDs work elsewhere. Agreed. [QMP and HMP code using bdrv_find() snipped] I think we should review with the QMP schema first, code second. Yes, get the interface right, and then it's easier to review the code that implements the interface. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] How to introduce bs-node_name ?
Hi list,
After a discussion on irc we have two potential solution in order to introduce
a new bs-node_name member in order to be able to manipulate the graph from the
monitors.
The first one is to make the QMP device parameter of the block commands optional
and add the node-name parameter as a second optional parameter.
This is Markus prefered solution and Eric is ok with making mandatory parameters
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 set 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 = qdict_get_str(qdict, device);
BlockDriverState *bs;
int ret;
if (!strcmp(device, all)) {
ret = bdrv_commit_all();
} else {
bs = bdrv_find(device);
if (!bs) {
monitor_printf(mon, Device '%s' not found\n, device);
return;
}
ret = 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 *device,
bool has_id,
const char *id,
bool has_name,
const char *name,
Error **errp)
{
BlockDriverState *bs = bdrv_find(device);
QEMUSnapshotInfo sn;
Error *local_err = NULL;
SnapshotInfo *info = 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 ==
TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
internal = common-action-blockdev_snapshot_internal_sync;
state = DO_UPCAST(InternalSnapshotState, common, common);
/* 1. parse input */
device = internal-device;
name = internal-name;
/* 2. check for validation */
bs = bdrv_find(device);
if (!bs) {
error_set(errp, QERR_DEVICE_NOT_FOUND, device);
return;
}
Drive backup
static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
{
DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
DriveBackup *backup;
Error *local_err = NULL;
assert(common-action-kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
backup = 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 = NULL;
state-job = NULL;
return;
}
state-bs = bdrv_find(backup-device);
state-job = state-bs-job;
}
Eject which should operate on backends
--
void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
{
BlockDriverState *bs;
bs = 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 **errp)
{
BlockDriverState *bs;
int err;
bs = bdrv_find(device);
if (!bs) {
error_set(errp, QERR_DEVICE_NOT_FOUND, device);
return;
}
err = bdrv_set_key(bs, password);
if (err == -EINVAL) {
error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
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,
Re: [Qemu-devel] How to introduce bs-node_name ?
On Mon, 10/28 16:40, Benoît Canet wrote:
Hi list,
After a discussion on irc we have two potential solution in order to introduce
a new bs-node_name member in order to be able to manipulate the graph from
the
monitors.
The first one is to make the QMP device parameter of the block commands
optional
and add the node-name parameter as a second optional parameter.
This is Markus prefered solution and Eric is ok with making mandatory
parameters
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 set 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 = qdict_get_str(qdict, device);
BlockDriverState *bs;
int ret;
if (!strcmp(device, all)) {
ret = bdrv_commit_all();
} else {
bs = bdrv_find(device);
if (!bs) {
monitor_printf(mon, Device '%s' not found\n, device);
return;
}
ret = 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 *device,
bool has_id,
const char *id,
bool has_name,
const char *name,
Error **errp)
{
BlockDriverState *bs = bdrv_find(device);
QEMUSnapshotInfo sn;
Error *local_err = NULL;
SnapshotInfo *info = 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 ==
TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
internal = common-action-blockdev_snapshot_internal_sync;
state = DO_UPCAST(InternalSnapshotState, common, common);
/* 1. parse input */
device = internal-device;
name = internal-name;
/* 2. check for validation */
bs = bdrv_find(device);
if (!bs) {
error_set(errp, QERR_DEVICE_NOT_FOUND, device);
return;
}
Drive backup
static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
{
DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
DriveBackup *backup;
Error *local_err = NULL;
assert(common-action-kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
backup = 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 = NULL;
state-job = NULL;
return;
}
state-bs = bdrv_find(backup-device);
state-job = state-bs-job;
}
Eject which should operate on backends
--
void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
{
BlockDriverState *bs;
bs = 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 **errp)
{
BlockDriverState *bs;
int err;
bs = bdrv_find(device);
if (!bs) {
error_set(errp, QERR_DEVICE_NOT_FOUND, device);
return;
}
err = bdrv_set_key(bs, password);
if (err == -EINVAL) {
error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
return;
} else if (err 0) {
error_set(errp, QERR_INVALID_PASSWORD);
