[Qemu-devel] [PATCH 0/2] Make open by reference use id then node-name

[Qemu-devel] [PATCH 0/2] Make open by reference use id then node-name

[Benoît Canet]

This series was asked by Kevin.
It sit on top of the snapshot fix patch and max openv2 series.

I have done the following tests.

check qemu-iotest ok
snapshot blockbackend by id ok
open quorum by node-name reference and id reference ok
Error messages seems ok

Benoît Canet (2):
  block: Relax bdrv_lookup_bs constraints.
  block: Open by reference will try device then node_name.

 block.c    | 36 +++++++++++++++++++-----------------
 blockdev.c |  6 ++++++
 2 files changed, 25 insertions(+), 17 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH 1/2] block: Relax bdrv_lookup_bs constraints.

[Benoît Canet]

The following patch will reuse bdrv_lookup_bs in order to open images by
references so the rules of usage of bdrv_lookup_bs must be relaxed a bit.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 3843f59..5ca901b 100644
--- a/block.c
+++ b/block.c
@@ -3579,30 +3579,26 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 {
     BlockDriverState *bs = NULL;
 
-    if ((!device && !node_name) || (device && node_name)) {
-        error_setg(errp, "Use either device or node-name but not both");
-        return NULL;
-    }
-
     if (device) {
         bs = bdrv_find(device);
 
-        if (!bs) {
-            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-            return NULL;
+        if (bs) {
+            return bs;
         }
-
-        return bs;
     }
 
-    bs = bdrv_find_node(node_name);
+    if (node_name) {
+        bs = bdrv_find_node(node_name);
 
-    if (!bs) {
-        error_set(errp, QERR_DEVICE_NOT_FOUND, node_name);
-        return NULL;
+        if (bs) {
+            return bs;
+        }
     }
 
-    return bs;
+    error_setg(errp, "Cannot find device=%s nor node_name=%s",
+                     device ? device : "",
+                     node_name ? node_name : "");
+    return NULL;
 }
 
 BlockDriverState *bdrv_next(BlockDriverState *bs)
-- 
1.8.3.2

[Qemu-devel] [PATCH 2/2] block: Open by reference will try device then node_name.

[Benoît Canet]

Since we introduced node_name for named bs of the graph modify the opening by
reference to use it as a fallback.

This patch also enforce the separation of the device id and graph node
namespaces.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c    | 10 ++++++++--
 blockdev.c |  6 ++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 5ca901b..af22e8a 100644
--- a/block.c
+++ b/block.c
@@ -796,6 +796,13 @@ static int bdrv_assign_node_name(BlockDriverState *bs,
         return -EINVAL;
     }
 
+    /* takes care of avoiding namespaces collisions */
+    if (bdrv_find(node_name)) {
+        error_setg(errp, "node-name=%s is conflicting with a device id",
+                   node_name);
+        return -EINVAL;
+    }
+
     /* takes care of avoiding duplicates node names */
     if (bdrv_find_node(node_name)) {
         error_setg(errp, "Duplicate node name");
@@ -1193,9 +1200,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
             return -EINVAL;
         }
 
-        bs = bdrv_find(reference);
+        bs = bdrv_lookup_bs(reference, reference, errp);
         if (!bs) {
-            error_setg(errp, "Cannot find block device '%s'", reference);
             return -ENODEV;
         }
         bdrv_ref(bs);
diff --git a/blockdev.c b/blockdev.c
index 7b7e349..5cd50f3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -463,6 +463,12 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
         }
     }
 
+    if (bdrv_find_node(qemu_opts_id(opts))) {
+        error_setg(errp, "device id=%s is conflicting with a node-name",
+                   qemu_opts_id(opts));
+        goto early_err;
+    }
+
     /* init */
     dinfo = g_malloc0(sizeof(*dinfo));
     dinfo->id = g_strdup(qemu_opts_id(opts));
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 1/2] block: Relax bdrv_lookup_bs constraints.

[Max Reitz]

On 12.02.2014 17:15, Benoît Canet wrote:
> The following patch will reuse bdrv_lookup_bs in order to open images by
> references so the rules of usage of bdrv_lookup_bs must be relaxed a bit.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>   block.c | 26 +++++++++++---------------
>   1 file changed, 11 insertions(+), 15 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] block: Open by reference will try device then node_name.

[Max Reitz]

On 12.02.2014 17:15, Benoît Canet wrote:
> Since we introduced node_name for named bs of the graph modify the opening by
> reference to use it as a fallback.
>
> This patch also enforce the separation of the device id and graph node
> namespaces.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>   block.c    | 10 ++++++++--
>   blockdev.c |  6 ++++++
>   2 files changed, 14 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 0/2] Make open by reference use id then node-name

[Kevin Wolf]

[ CC Stefan ]

Am 12.02.2014 um 17:15 hat Benoît Canet geschrieben:
> This series was asked by Kevin.
> It sit on top of the snapshot fix patch and max openv2 series.
> 
> I have done the following tests.
> 
> check qemu-iotest ok
> snapshot blockbackend by id ok
> open quorum by node-name reference and id reference ok
> Error messages seems ok
> 
> Benoît Canet (2):
>   block: Relax bdrv_lookup_bs constraints.
>   block: Open by reference will try device then node_name.

Very nice! Haven't reviewed it in detail, but you got the review from
Max, which should be good enough for Stefan to merge it.

Kevin

Re: [Qemu-devel] [PATCH 0/2] Make open by reference use id then node-name

[Stefan Hajnoczi]

On Wed, Feb 12, 2014 at 05:15:05PM +0100, Benoît Canet wrote:
> This series was asked by Kevin.
> It sit on top of the snapshot fix patch and max openv2 series.
> 
> I have done the following tests.
> 
> check qemu-iotest ok
> snapshot blockbackend by id ok
> open quorum by node-name reference and id reference ok
> Error messages seems ok
> 
> Benoît Canet (2):
>   block: Relax bdrv_lookup_bs constraints.
>   block: Open by reference will try device then node_name.
> 
>  block.c    | 36 +++++++++++++++++++-----------------
>  blockdev.c |  6 ++++++
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> -- 
> 1.8.3.2
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan