[Qemu-devel] [PATCH 0/4] Block-related miscellaneous cleanups

[Qemu-devel] [PATCH 0/4] Block-related miscellaneous cleanups

[Markus Armbruster]

Random crap I ran into while working on a bigger task.

Markus Armbruster (4):
  qemu-io: Clean up openfile() after commit 2e40134
  xen_disk: Plug memory leak on error path
  xen: Drop redundant bdrv_close() from pci_piix3_xen_ide_unplug()
  thread-pool: Drop unnecessary includes

 hw/block/xen_disk.c         | 31 ++++++++++++++-----------------
 hw/ide/piix.c               |  1 -
 include/block/thread-pool.h |  6 +-----
 qemu-io.c                   | 34 ++++++++++++----------------------
 thread-pool.c               |  1 -
 5 files changed, 27 insertions(+), 46 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/4] qemu-io: Clean up openfile() after commit 2e40134

[Markus Armbruster]

Commit 6db9560 split off the growable case so it can use
bdrv_file_open() instead of bdrv_open() then.  Growable BDSes become
anonymous.  Weird.

Commit 2e40134 folded bdrv_file_open() back into bdrv_open() with new
flag BDRV_O_PROTOCOL.  We still have two bdrv_open() calls, and
growable BDSes remain anonymous.

Circle back to before commit 6db9560: just one call, not anonymous.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-io.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index 33c96c4..d2ab694 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -58,30 +58,20 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
         return 1;
     }
 
+    qemuio_bs = bdrv_new("hda", &error_abort);
+
     if (growable) {
-        if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL,
-                      NULL, &local_err))
-        {
-            fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
-                    name ? " device " : "", name ?: "",
-                    error_get_pretty(local_err));
-            error_free(local_err);
-            return 1;
-        }
-    } else {
-        qemuio_bs = bdrv_new("hda", &error_abort);
+        flags |= BDRV_O_PROTOCOL;
+    }
 
-        if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &local_err)
-            < 0)
-        {
-            fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
-                    name ? " device " : "", name ?: "",
-                    error_get_pretty(local_err));
-            error_free(local_err);
-            bdrv_unref(qemuio_bs);
-            qemuio_bs = NULL;
-            return 1;
-        }
+    if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &local_err) < 0) {
+        fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
+                name ? " device " : "", name ?: "",
+                error_get_pretty(local_err));
+        error_free(local_err);
+        bdrv_unref(qemuio_bs);
+        qemuio_bs = NULL;
+        return 1;
     }
 
     return 0;
-- 
1.9.3

[Qemu-devel] [PATCH 2/4] xen_disk: Plug memory leak on error path

[Markus Armbruster]

While there, streamline control flow a bit.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block/xen_disk.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index a221d0b..2dcef07 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -852,28 +852,25 @@ static int blk_connect(struct XenDevice *xendev)
     blkdev->dinfo = drive_get(IF_XEN, 0, index);
     if (!blkdev->dinfo) {
         Error *local_err = NULL;
+        BlockDriver *drv;
+
         /* setup via xenbus -> create new block driver instance */
         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
-        blkdev->bs = bdrv_new(blkdev->dev, &local_err);
-        if (local_err) {
-            blkdev->bs = NULL;
-        }
-        if (blkdev->bs) {
-            BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
-                                                           readonly);
-            if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, qflags,
-                          drv, &local_err) != 0)
-            {
-                xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
-                              error_get_pretty(local_err));
-                error_free(local_err);
-                bdrv_unref(blkdev->bs);
-                blkdev->bs = NULL;
-            }
-        }
+        blkdev->bs = bdrv_new(blkdev->dev, NULL);
         if (!blkdev->bs) {
             return -1;
         }
+
+        drv = bdrv_find_whitelisted_format(blkdev->fileproto, readonly);
+        if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, qflags,
+                      drv, &local_err) != 0) {
+            xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
+                          error_get_pretty(local_err));
+            error_free(local_err);
+            bdrv_unref(blkdev->bs);
+            blkdev->bs = NULL;
+            return -1;
+        }
     } else {
         /* setup via qemu cmdline -> already setup for us */
         xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
-- 
1.9.3

[Qemu-devel] [PATCH 3/4] xen: Drop redundant bdrv_close() from pci_piix3_xen_ide_unplug()

[Markus Armbruster]

drive_del() closes just fine.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/piix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 59319eb..49e78a7 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -182,7 +182,6 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
             if (ds) {
                 bdrv_detach_dev(di->bdrv, ds);
             }
-            bdrv_close(di->bdrv);
             pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
             drive_del(di);
         }
-- 
1.9.3

[Qemu-devel] [PATCH 4/4] thread-pool: Drop unnecessary includes

[Markus Armbruster]

Dragging block_int.h into a header is *not* nice.  Fortunately, this
is the only offender.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/block/thread-pool.h | 6 +-----
 thread-pool.c               | 1 -
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h
index 32afcdd..4723752 100644
--- a/include/block/thread-pool.h
+++ b/include/block/thread-pool.h
@@ -18,11 +18,7 @@
 #ifndef QEMU_THREAD_POOL_H
 #define QEMU_THREAD_POOL_H 1
 
-#include "qemu-common.h"
-#include "qemu/queue.h"
-#include "qemu/thread.h"
-#include "block/coroutine.h"
-#include "block/block_int.h"
+#include "block/block.h"
 
 typedef int ThreadPoolFunc(void *opaque);
 
diff --git a/thread-pool.c b/thread-pool.c
index 23888dc..bc07d7a 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -20,7 +20,6 @@
 #include "qemu/osdep.h"
 #include "block/coroutine.h"
 #include "trace.h"
-#include "block/block_int.h"
 #include "block/thread-pool.h"
 #include "qemu/main-loop.h"
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/4] Block-related miscellaneous cleanups

[Kevin Wolf]

Am 08.09.2014 um 18:50 hat Markus Armbruster geschrieben:
> Random crap I ran into while working on a bigger task.
> 
> Markus Armbruster (4):
>   qemu-io: Clean up openfile() after commit 2e40134
>   xen_disk: Plug memory leak on error path
>   xen: Drop redundant bdrv_close() from pci_piix3_xen_ide_unplug()
>   thread-pool: Drop unnecessary includes

Thanks, applied all to the block branch.

Kevin