* [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend
@ 2016-02-23 17:16 Kevin Wolf
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Kevin Wolf @ 2016-02-23 17:16 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, armbru, mreitz
See patch 1 for the detailed description why this is needed. The short
version is that libvirt needs to enable detect-zeroes on a mirror
target.
Peter, can you please check if this provides what you need? I checked
whether I could do the mirroring operation manually; and if I did it
right, it looks good to me. (Shortened) protocol of my test:
$ ./qemu-img create -f qcow2 /tmp/test.qcow2 64M
Formatting '/tmp/test.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-io -c 'write -P 0 0 64M' /tmp/test.qcow2
wrote 67108864/67108864 bytes at offset 0
64 MiB, 1 ops; 0.3141 sec (203.694 MiB/sec and 3.1827 ops/sec)
$ ls -lh /tmp/test.qcow2
-rw-r--r--. 1 kwolf kwolf 65M 23. Feb 17:57 /tmp/test.qcow2
$ ./qemu-img create -f qcow2 /tmp/copy.qcow2 64M
Formatting '/tmp/copy.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ x86_64-softmmu/qemu-system-x86_64 -qmp stdio -hda /tmp/test.qcow2
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 5, "major": 2}, "package": ""}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"human-monitor-command","arguments":{"command-line":"drive_add -n 0 file.filename=/tmp/copy.qcow2,node-name=copy,detect-zeroes=on"}}
{"return": ""}
{"execute":"blockdev-mirror","arguments":{"device":"ide0-hd0","target":"copy","sync":"full"}}
{"return": {}}
{"timestamp": {"seconds": 1456247099, "microseconds": 895774}, "event": "BLOCK_JOB_READY", "data": {"device": "ide0-hd0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "mirror"}}
{"execute":"block-job-complete","arguments":{"device":"ide0-hd0"}}
{"return": {}}
{"timestamp": {"seconds": 1456247127, "microseconds": 552227}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "ide0-hd0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "mirror"}}
$ ls -lh /tmp/copy.qcow2
-rw-r--r--. 1 kwolf kwolf 320K 23. Feb 18:04 /tmp/copy.qcow2
Kevin Wolf (2):
hmp: 'drive_add -n' for creating a node without BB
hmp: Extend drive_del to delete nodes without BB
blockdev.c | 39 +++++++++++++++++++++++++++++++++++++++
device-hotplug.c | 7 +++++++
hmp-commands.hx | 4 ++--
include/block/block_int.h | 2 ++
4 files changed, 50 insertions(+), 2 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB
2016-02-23 17:16 [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
@ 2016-02-23 17:16 ` Kevin Wolf
2016-02-24 17:50 ` Max Reitz
2016-02-25 13:18 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2016-02-23 17:16 ` [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes " Kevin Wolf
2016-03-09 10:54 ` [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
2 siblings, 2 replies; 13+ messages in thread
From: Kevin Wolf @ 2016-02-23 17:16 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, armbru, mreitz
This patch adds an option to the drive_add HMP command to create only a
BlockDriverState without a BlockBackend on top.
The motivation for this is that libvirt needs to specify options to a
migration target (specifically, detect-zeroes). drive-mirror doesn't
allow specifying options, and the proper way to do this is to create the
target BDS separately with blockdev-add (where you can specify options)
and then use blockdev-mirror to that BDS.
However, libvirt can't use blockdev-add as long as it is still
experimental, and we're expecting that it will still take some time, so
we need to resort to drive_add.
The problem with drive_add is that so far it always created a BB, and
BDSes with a BB can't be used as a mirroring target as long as we don't
support multiple BBs per BDS - and while we're working towards that
goal, it's another thing that will still take some time.
So to achieve the goal, the simplest solution to provide the
functionality now without adding one-off options to the mirror QMP
commands is to extend drive_add to create nodes without BBs.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 30 ++++++++++++++++++++++++++++++
device-hotplug.c | 7 +++++++
hmp-commands.hx | 4 ++--
include/block/block_int.h | 2 ++
4 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index d4bc435..3f46bc1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3867,6 +3867,36 @@ out:
aio_context_release(aio_context);
}
+void hmp_drive_add_node(Monitor *mon, const char *optstr)
+{
+ QemuOpts *opts;
+ QDict *qdict;
+ Error *local_err = NULL;
+
+ opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
+ if (!opts) {
+ return;
+ }
+
+ qdict = qemu_opts_to_qdict(opts, NULL);
+
+ if (!qdict_get_try_str(qdict, "node-name")) {
+ error_report("'node-name' needs to be specified");
+ goto out;
+ }
+
+ BlockDriverState *bs = bds_tree_init(qdict, &local_err);
+ if (!bs) {
+ error_report_err(local_err);
+ goto out;
+ }
+
+ QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+
+out:
+ qemu_opts_del(opts);
+}
+
void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
{
QmpOutputVisitor *ov = qmp_output_visitor_new();
diff --git a/device-hotplug.c b/device-hotplug.c
index 9a7cd66..3e5cdaa 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -30,6 +30,7 @@
#include "qemu/config-file.h"
#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
+#include "block/block_int.h"
static DriveInfo *add_init_drive(const char *optstr)
{
@@ -55,6 +56,12 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
{
DriveInfo *dinfo = NULL;
const char *opts = qdict_get_str(qdict, "opts");
+ bool node = qdict_get_try_bool(qdict, "node", false);
+
+ if (node) {
+ hmp_drive_add_node(mon, opts);
+ return;
+ }
dinfo = add_init_drive(opts);
if (!dinfo) {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..3b44e52 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1200,8 +1200,8 @@ ETEXI
{
.name = "drive_add",
- .args_type = "pci_addr:s,opts:s",
- .params = "[[<domain>:]<bus>:]<slot>\n"
+ .args_type = "node:-n,pci_addr:s,opts:s",
+ .params = "[-n] [[<domain>:]<bus>:]<slot>\n"
"[file=file][,if=type][,bus=n]\n"
"[,unit=m][,media=d][,index=i]\n"
"[,cyls=c,heads=h,secs=s[,trans=t]]\n"
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9ef823a..dda5ba0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -694,6 +694,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
BlockCompletionFunc *cb, void *opaque,
BlockJobTxn *txn, Error **errp);
+void hmp_drive_add_node(Monitor *mon, const char *optstr);
+
void blk_set_bs(BlockBackend *blk, BlockDriverState *bs);
void blk_dev_change_media_cb(BlockBackend *blk, bool load);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-23 17:16 [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
@ 2016-02-23 17:16 ` Kevin Wolf
2016-02-24 17:54 ` Max Reitz
2016-03-09 10:54 ` [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
2 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2016-02-23 17:16 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, armbru, mreitz
Now that we can use drive_add to create new nodes without a BB, we also
want to be able to delete such nodes again.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 3f46bc1..b76b6cd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2816,6 +2816,15 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
AioContext *aio_context;
Error *local_err = NULL;
+ bs = bdrv_find_node(id);
+ if (bs) {
+ qmp_x_blockdev_del(false, NULL, true, id, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
+ return;
+ }
+
blk = blk_by_name(id);
if (!blk) {
error_report("Device '%s' not found", id);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
@ 2016-02-24 17:50 ` Max Reitz
2016-02-24 18:24 ` Kevin Wolf
2016-02-25 13:18 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
1 sibling, 1 reply; 13+ messages in thread
From: Max Reitz @ 2016-02-24 17:50 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: pkrempa, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2364 bytes --]
On 23.02.2016 18:16, Kevin Wolf wrote:
> This patch adds an option to the drive_add HMP command to create only a
> BlockDriverState without a BlockBackend on top.
>
> The motivation for this is that libvirt needs to specify options to a
> migration target (specifically, detect-zeroes). drive-mirror doesn't
> allow specifying options, and the proper way to do this is to create the
> target BDS separately with blockdev-add (where you can specify options)
> and then use blockdev-mirror to that BDS.
>
> However, libvirt can't use blockdev-add as long as it is still
> experimental, and we're expecting that it will still take some time, so
> we need to resort to drive_add.
>
> The problem with drive_add is that so far it always created a BB, and
> BDSes with a BB can't be used as a mirroring target as long as we don't
> support multiple BBs per BDS - and while we're working towards that
> goal, it's another thing that will still take some time.
>
> So to achieve the goal, the simplest solution to provide the
> functionality now without adding one-off options to the mirror QMP
> commands is to extend drive_add to create nodes without BBs.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 30 ++++++++++++++++++++++++++++++
> device-hotplug.c | 7 +++++++
> hmp-commands.hx | 4 ++--
> include/block/block_int.h | 2 ++
> 4 files changed, 41 insertions(+), 2 deletions(-)
>
Patch looks good to me (well, except for it being a pity we have to fall
back on this HMP command), I only have a minor suggestion:
[...]
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index bb52e4d..3b44e52 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1200,8 +1200,8 @@ ETEXI
>
> {
> .name = "drive_add",
> - .args_type = "pci_addr:s,opts:s",
> - .params = "[[<domain>:]<bus>:]<slot>\n"
> + .args_type = "node:-n,pci_addr:s,opts:s",
> + .params = "[-n] [[<domain>:]<bus>:]<slot>\n"
> "[file=file][,if=type][,bus=n]\n"
> "[,unit=m][,media=d][,index=i]\n"
> "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
The description reads:
> Add drive to PCI storage controller.
Maybe this should be extended now?
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-23 17:16 ` [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes " Kevin Wolf
@ 2016-02-24 17:54 ` Max Reitz
2016-02-24 18:23 ` Kevin Wolf
2016-02-25 12:51 ` Peter Krempa
0 siblings, 2 replies; 13+ messages in thread
From: Max Reitz @ 2016-02-24 17:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: pkrempa, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]
On 23.02.2016 18:16, Kevin Wolf wrote:
> Now that we can use drive_add to create new nodes without a BB, we also
> want to be able to delete such nodes again.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 3f46bc1..b76b6cd 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2816,6 +2816,15 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
> AioContext *aio_context;
> Error *local_err = NULL;
>
> + bs = bdrv_find_node(id);
> + if (bs) {
> + qmp_x_blockdev_del(false, NULL, true, id, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + }
> + return;
> + }
> +
> blk = blk_by_name(id);
> if (!blk) {
> error_report("Device '%s' not found", id);
>
It's a bit strange to require the user to specify the node name using
"node-name" for drive_add, but the to use "id" in drive_del; especially
because x-blockdev-del uses "node-name", too.
Up to your discretion.
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-24 17:54 ` Max Reitz
@ 2016-02-24 18:23 ` Kevin Wolf
2016-02-26 13:09 ` Max Reitz
2016-02-25 12:51 ` Peter Krempa
1 sibling, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2016-02-24 18:23 UTC (permalink / raw)
To: Max Reitz; +Cc: pkrempa, armbru, qemu-block, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]
Am 24.02.2016 um 18:54 hat Max Reitz geschrieben:
> On 23.02.2016 18:16, Kevin Wolf wrote:
> > Now that we can use drive_add to create new nodes without a BB, we also
> > want to be able to delete such nodes again.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > blockdev.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 3f46bc1..b76b6cd 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -2816,6 +2816,15 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
> > AioContext *aio_context;
> > Error *local_err = NULL;
> >
> > + bs = bdrv_find_node(id);
> > + if (bs) {
> > + qmp_x_blockdev_del(false, NULL, true, id, &local_err);
> > + if (local_err) {
> > + error_report_err(local_err);
> > + }
> > + return;
> > + }
> > +
> > blk = blk_by_name(id);
> > if (!blk) {
> > error_report("Device '%s' not found", id);
> >
>
> It's a bit strange to require the user to specify the node name using
> "node-name" for drive_add, but the to use "id" in drive_del; especially
> because x-blockdev-del uses "node-name", too.
Not sure I understand. For the user of drive_del that's simply a
positional parameter, so they use neither "id" nor "node-name". Am I
missing something?
Kevin
> Up to your discretion.
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB
2016-02-24 17:50 ` Max Reitz
@ 2016-02-24 18:24 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2016-02-24 18:24 UTC (permalink / raw)
To: Max Reitz; +Cc: pkrempa, armbru, qemu-block, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2640 bytes --]
Am 24.02.2016 um 18:50 hat Max Reitz geschrieben:
> On 23.02.2016 18:16, Kevin Wolf wrote:
> > This patch adds an option to the drive_add HMP command to create only a
> > BlockDriverState without a BlockBackend on top.
> >
> > The motivation for this is that libvirt needs to specify options to a
> > migration target (specifically, detect-zeroes). drive-mirror doesn't
> > allow specifying options, and the proper way to do this is to create the
> > target BDS separately with blockdev-add (where you can specify options)
> > and then use blockdev-mirror to that BDS.
> >
> > However, libvirt can't use blockdev-add as long as it is still
> > experimental, and we're expecting that it will still take some time, so
> > we need to resort to drive_add.
> >
> > The problem with drive_add is that so far it always created a BB, and
> > BDSes with a BB can't be used as a mirroring target as long as we don't
> > support multiple BBs per BDS - and while we're working towards that
> > goal, it's another thing that will still take some time.
> >
> > So to achieve the goal, the simplest solution to provide the
> > functionality now without adding one-off options to the mirror QMP
> > commands is to extend drive_add to create nodes without BBs.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > blockdev.c | 30 ++++++++++++++++++++++++++++++
> > device-hotplug.c | 7 +++++++
> > hmp-commands.hx | 4 ++--
> > include/block/block_int.h | 2 ++
> > 4 files changed, 41 insertions(+), 2 deletions(-)
> >
>
> Patch looks good to me (well, except for it being a pity we have to fall
> back on this HMP command), I only have a minor suggestion:
>
> [...]
>
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index bb52e4d..3b44e52 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1200,8 +1200,8 @@ ETEXI
> >
> > {
> > .name = "drive_add",
> > - .args_type = "pci_addr:s,opts:s",
> > - .params = "[[<domain>:]<bus>:]<slot>\n"
> > + .args_type = "node:-n,pci_addr:s,opts:s",
> > + .params = "[-n] [[<domain>:]<bus>:]<slot>\n"
> > "[file=file][,if=type][,bus=n]\n"
> > "[,unit=m][,media=d][,index=i]\n"
> > "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
>
> The description reads:
>
> > Add drive to PCI storage controller.
>
> Maybe this should be extended now?
It was already wrong before this patch, but I guess I could just add
another patch to the series anyway.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-24 17:54 ` Max Reitz
2016-02-24 18:23 ` Kevin Wolf
@ 2016-02-25 12:51 ` Peter Krempa
2016-02-26 13:11 ` Max Reitz
1 sibling, 1 reply; 13+ messages in thread
From: Peter Krempa @ 2016-02-25 12:51 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, armbru, qemu-block, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On Wed, Feb 24, 2016 at 18:54:45 +0100, Max Reitz wrote:
> On 23.02.2016 18:16, Kevin Wolf wrote:
> > Now that we can use drive_add to create new nodes without a BB, we also
> > want to be able to delete such nodes again.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > blockdev.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
[..]
> >
>
> It's a bit strange to require the user to specify the node name using
> "node-name" for drive_add, but the to use "id" in drive_del; especially
> because x-blockdev-del uses "node-name", too.
Well, since 'x-blockdev-del' is considered unstable yet I can't really
use it in libvirt, thus we discussed using drive_del as a workaround.
It makes partially sense since we'd add the new node with 'drive_add' in
the first place.
Peter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
2016-02-24 17:50 ` Max Reitz
@ 2016-02-25 13:18 ` Alberto Garcia
2016-02-25 16:05 ` Eric Blake
1 sibling, 1 reply; 13+ messages in thread
From: Alberto Garcia @ 2016-02-25 13:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, armbru
On Tue 23 Feb 2016 06:16:38 PM CET, Kevin Wolf wrote:
> However, libvirt can't use blockdev-add as long as it is still
> experimental, and we're expecting that it will still take some time,
> so we need to resort to drive_add.
But how stable is the HMP API supposed to be?
Berto
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB
2016-02-25 13:18 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
@ 2016-02-25 16:05 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2016-02-25 16:05 UTC (permalink / raw)
To: Alberto Garcia, Kevin Wolf, qemu-block; +Cc: armbru, qemu-devel, mreitz
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
On 02/25/2016 06:18 AM, Alberto Garcia wrote:
> On Tue 23 Feb 2016 06:16:38 PM CET, Kevin Wolf wrote:
>
>> However, libvirt can't use blockdev-add as long as it is still
>> experimental, and we're expecting that it will still take some time,
>> so we need to resort to drive_add.
>
> But how stable is the HMP API supposed to be?
In general, it's not stable. But for this particular API, as long as we
don't have a fully-working QMP replacement, we've been careful to keep
HMP working unchanged.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-24 18:23 ` Kevin Wolf
@ 2016-02-26 13:09 ` Max Reitz
0 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2016-02-26 13:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pkrempa, armbru, qemu-block, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1591 bytes --]
On 24.02.2016 19:23, Kevin Wolf wrote:
> Am 24.02.2016 um 18:54 hat Max Reitz geschrieben:
>> On 23.02.2016 18:16, Kevin Wolf wrote:
>>> Now that we can use drive_add to create new nodes without a BB, we also
>>> want to be able to delete such nodes again.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>> blockdev.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index 3f46bc1..b76b6cd 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -2816,6 +2816,15 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
>>> AioContext *aio_context;
>>> Error *local_err = NULL;
>>>
>>> + bs = bdrv_find_node(id);
>>> + if (bs) {
>>> + qmp_x_blockdev_del(false, NULL, true, id, &local_err);
>>> + if (local_err) {
>>> + error_report_err(local_err);
>>> + }
>>> + return;
>>> + }
>>> +
>>> blk = blk_by_name(id);
>>> if (!blk) {
>>> error_report("Device '%s' not found", id);
>>>
>>
>> It's a bit strange to require the user to specify the node name using
>> "node-name" for drive_add, but the to use "id" in drive_del; especially
>> because x-blockdev-del uses "node-name", too.
>
> Not sure I understand. For the user of drive_del that's simply a
> positional parameter, so they use neither "id" nor "node-name". Am I
> missing something?
No, it's just me being confused again by the way HMP works. I keep
forgetting that the user doesn't specify the parameter names. So the R-b
stands. :-)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes without BB
2016-02-25 12:51 ` Peter Krempa
@ 2016-02-26 13:11 ` Max Reitz
0 siblings, 0 replies; 13+ messages in thread
From: Max Reitz @ 2016-02-26 13:11 UTC (permalink / raw)
To: Peter Krempa; +Cc: Kevin Wolf, armbru, qemu-block, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1145 bytes --]
On 25.02.2016 13:51, Peter Krempa wrote:
> On Wed, Feb 24, 2016 at 18:54:45 +0100, Max Reitz wrote:
>> On 23.02.2016 18:16, Kevin Wolf wrote:
>>> Now that we can use drive_add to create new nodes without a BB, we also
>>> want to be able to delete such nodes again.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>> blockdev.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>
> [..]
>
>>>
>>
>> It's a bit strange to require the user to specify the node name using
>> "node-name" for drive_add, but the to use "id" in drive_del; especially
>> because x-blockdev-del uses "node-name", too.
>
> Well, since 'x-blockdev-del' is considered unstable yet I can't really
> use it in libvirt, thus we discussed using drive_del as a workaround.
>
> It makes partially sense since we'd add the new node with 'drive_add' in
> the first place.
Yes, you're right. However, my question was solely about the parameter
name (reusing "id" for a node name). As Kevin replied, for HMP the
parameter name doesn't really matter to the user anyway, so reusing the
"id" parameter here is completely fine.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend
2016-02-23 17:16 [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
2016-02-23 17:16 ` [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes " Kevin Wolf
@ 2016-03-09 10:54 ` Kevin Wolf
2 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2016-03-09 10:54 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, pkrempa, armbru, mreitz
Am 23.02.2016 um 18:16 hat Kevin Wolf geschrieben:
> See patch 1 for the detailed description why this is needed. The short
> version is that libvirt needs to enable detect-zeroes on a mirror
> target.
>
> Peter, can you please check if this provides what you need? I checked
> whether I could do the mirroring operation manually; and if I did it
> right, it looks good to me. (Shortened) protocol of my test:
> [...]
Applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-03-09 10:54 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-23 17:16 [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
2016-02-23 17:16 ` [Qemu-devel] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB Kevin Wolf
2016-02-24 17:50 ` Max Reitz
2016-02-24 18:24 ` Kevin Wolf
2016-02-25 13:18 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2016-02-25 16:05 ` Eric Blake
2016-02-23 17:16 ` [Qemu-devel] [PATCH 2/2] hmp: Extend drive_del to delete nodes " Kevin Wolf
2016-02-24 17:54 ` Max Reitz
2016-02-24 18:23 ` Kevin Wolf
2016-02-26 13:09 ` Max Reitz
2016-02-25 12:51 ` Peter Krempa
2016-02-26 13:11 ` Max Reitz
2016-03-09 10:54 ` [Qemu-devel] [PATCH 0/2] block: drive_add/del without BlockBackend Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).