Patchwork [v3] xen_disk: cope with missing xenstore "params" node

login
register
mail settings
Submitter Stefano Stabellini
Date June 27, 2011, 3:10 p.m.
Message ID <1309187401-27685-1-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/102200/
State New
Headers show

Comments

Stefano Stabellini - June 27, 2011, 3:10 p.m.
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

When disk is a cdrom and the drive is empty the "params" node in
xenstore might be missing completely: cope with it instead of
segfaulting.


Updated in v2:

- actually removed the strchr(blkdev->params, ':') that caused the
segfault;

- free all the allocated strings from xenstore before returning;


Updated in v3:

- set blkdev fields to NULL after free'ing them.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xen_disk.c |   34 +++++++++++++++++++++++++++-------
 1 files changed, 27 insertions(+), 7 deletions(-)
Alexander Graf - June 30, 2011, 3:11 p.m.
On 06/27/2011 05:10 PM, stefano.stabellini@eu.citrix.com wrote:
> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>
> When disk is a cdrom and the drive is empty the "params" node in
> xenstore might be missing completely: cope with it instead of
> segfaulting.
>
>
> Updated in v2:
>
> - actually removed the strchr(blkdev->params, ':') that caused the
> segfault;
>
> - free all the allocated strings from xenstore before returning;
>
>
> Updated in v3:
>
> - set blkdev fields to NULL after free'ing them.

Thanks, applied to xen-next branch.


Alex

Patch

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 096d1c9..faaa065 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -616,12 +616,14 @@  static int blk_init(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     int index, qflags, have_barriers, info = 0;
-    char *h;
 
     /* read xenstore entries */
     if (blkdev->params == NULL) {
+        char *h = NULL;
         blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
-        h = strchr(blkdev->params, ':');
+        if (blkdev->params != NULL) {
+            h = strchr(blkdev->params, ':');
+        }
         if (h != NULL) {
             blkdev->fileproto = blkdev->params;
             blkdev->filename  = h+1;
@@ -649,7 +651,7 @@  static int blk_init(struct XenDevice *xendev)
         blkdev->mode == NULL   ||
         blkdev->type == NULL   ||
         blkdev->dev == NULL) {
-        return -1;
+        goto out_error;
     }
 
     /* read-only ? */
@@ -672,10 +674,15 @@  static int blk_init(struct XenDevice *xendev)
         /* 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);
-        if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
-                      bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) {
-            bdrv_delete(blkdev->bs);
-            return -1;
+        if (blkdev->bs) {
+            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
+                        bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) {
+                bdrv_delete(blkdev->bs);
+                blkdev->bs = NULL;
+            }
+        }
+        if (!blkdev->bs) {
+            goto out_error;
         }
     } else {
         /* setup via qemu cmdline -> already setup for us */
@@ -704,6 +711,19 @@  static int blk_init(struct XenDevice *xendev)
     xenstore_write_be_int(&blkdev->xendev, "sectors",
                           blkdev->file_size / blkdev->file_blk);
     return 0;
+
+out_error:
+    qemu_free(blkdev->params);
+    blkdev->params = NULL;
+    qemu_free(blkdev->mode);
+    blkdev->mode = NULL;
+    qemu_free(blkdev->type);
+    blkdev->type = NULL;
+    qemu_free(blkdev->dev);
+    blkdev->dev = NULL;
+    qemu_free(blkdev->devtype);
+    blkdev->devtype = NULL;
+    return -1;
 }
 
 static int blk_connect(struct XenDevice *xendev)