Skip to contentSkip to conversation

tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

PATCH In review Ian Campbell 4 participants 13 messages Mar 31, 2011 – Apr 5, 2011

lore
Share & tools
lore

Ian Campbell

author
#0 lore raw

PATCH 1/1 tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301571089 -3600
# Node ID d0dd569bfcb0f5f8e2d903c8b6f9999ff1290e96 # Parent 7b5c5a365f2a0a57e83479f69e5b56beb07752c1
tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

xvde+ are ignored.

Old qemu did this itself internally. Fixes "qemu: -xvda: invalid option" and allows PVHVM to work with new qemu.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl_dm.c +20 -2
diff -r 7b5c5a365f2a -r d0dd569bfcb0 tools/libxl/libxl_dm.c
@@ -175,6 +175,7 @@ static char ** libxl__build_device_model
                                                   libxl_device_nic *vifs,
                                                   int num_vifs)
 {
+    libxl_ctx *ctx = libxl__gc_owner(gc);
     flexarray_t *dm_args;
     libxl_device_disk *disks;
     int nb, i;

@@ -318,8 +319,25 @@ static char ** libxl__build_device_model
                 flexarray_append(dm_args, "-cdrom");
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                char hd_dev[] = "hdX";
+                const char *vdev = NULL;
+
+                if (strncmp(disks[i].vdev, "xvd", 3) == 0) {
+                    if (disks[i].vdev[3] >= 'a' && disks[i].vdev[3] <= 'd') {
+                        hd_dev[2] = disks[i].vdev[3];
+                        vdev = &hd_dev[0];
+                        LIBXL__LOG(ctx, LIBXL__LOG_INFO, "translated disk device %s to %s", disks[i].vdev, vdev);
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_INFO, "ignored disk device %s", disks[i].vdev);
+                    }
+                } else {
+                    vdev = disks[i].vdev;
+                }
+
+                if (vdev) {
+                    flexarray_append(dm_args, libxl__sprintf(gc, "-%s", vdev));
+                    flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                }
             }
             libxl_device_disk_destroy(&disks[i]);
         }
#1 lore raw

Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):

Quoted 3 lines

tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

xvde+ are ignored.

I don't think this is strictly speaking correct. According to the spec doc/misc/vbd-interface.txt, we are supposed to support vdevs in a variety of formats.

What this code ought to be doing is using the parsing function and then reconstituting the vdev name for qemu's benefit.

Ian.

#2 lore raw
Quoted 11 lines · Ian Jackson

Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):

Quoted 3 lines

tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

xvde+ are ignored.

I don't think this is strictly speaking correct. According to the spec doc/misc/vbd-interface.txt, we are supposed to support vdevs in a variety of formats.

What this code ought to be doing is using the parsing function and then reconstituting the vdev name for qemu's benefit.

makes sense, I'd forgotten about the parsing function.

I guess for everything other than the sdX syntax I should translate the result of libxl__device_disk_dev_number into an -hd[a-d] for values < 4 and ignore anything greater, but for sdX syntax I want to pass it through as is? And is the limit 4 or 16 in that case? (I'm not 100% sure of the old qemu's behaviour...)

Ian

author
#3 lore raw
Quoted 13 lines · Ian Campbell
Quoted 11 lines · Ian Jackson

Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):

Quoted 3 lines

tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu

xvde+ are ignored.

I don't think this is strictly speaking correct. According to the spec doc/misc/vbd-interface.txt, we are supposed to support vdevs in a variety of formats.

What this code ought to be doing is using the parsing function and then reconstituting the vdev name for qemu's benefit.

Quoted 7 lines

makes sense, I'd forgotten about the parsing function.

I guess for everything other than the sdX syntax I should translate the result of libxl__device_disk_dev_number into an -hd[a-d] for values < 4 and ignore anything greater, but for sdX syntax I want to pass it through as is? And is the limit 4 or 16 in that case? (I'm not 100% sure of the old qemu's behaviour...)

qemu-xen translates up to MAX_SCSI_DEVS into sdX devices, MAX_SCSI_DEVS is 7 in qemu-xen and 256 in upstream qemu, so I think for xl I'll just translate sd devices with no limits -- adding more sd devices than the particular qemu can cope with seems like user error to me.

8<------------------------------------------

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301663315 -3600
# Node ID cd7f020e44b051542b82d54b8a4ef526cf403608
# Parent 356becdb49de5ac69f174ec8d0804882fef1ef3c
tools: libxl: translate disk device names to hd[a-d] for new qemu

We convert the first four non-SCSI disks to hd[a-d] and ignore any additional non-SCSI disks.

SCSI disks are passed through as is. qemu-xen was limited to 7 SCSI devices but upstream supports 256.

qemu-xen did all this itself internally.

Fixes "qemu: -xvda: invalid option" and allows PVHVM to work with upstream qemu.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl_dm.c +21 -2
diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c
@@ -318,8 +318,27 @@ static char ** libxl__build_device_model
                 flexarray_append(dm_args, "-cdrom");
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                int dev_number = libxl__device_disk_dev_number(disks[i].vdev);
+                char *vdev;
+
+                /* Explicit sd disks are passed through as is.
+                 *
+                 * For other disks we translate devices 0..3 into
+                 * hd[a-d] and ignore the rest.
+                 */
+                if (dev_number == -1)
+                    vdev = NULL;
+                else if (strncmp(disks[i].vdev, "sd", 2) == 0)
+                    vdev = libxl__sprintf(gc, "-%s", vdev);
+                else if (dev_number < 4)
+                    vdev = libxl__sprintf(gc, "-hd%c", 'a' + dev_number);
+                else
+                    vdev = NULL;
+
+                if (vdev) {
+                    flexarray_append(dm_args, libxl__sprintf(gc, "-%s", vdev));
+                    flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                }
             }
             libxl_device_disk_destroy(&disks[i]);
         }
#4 lore raw
Quoted 43 lines · Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301663315 -3600
# Node ID cd7f020e44b051542b82d54b8a4ef526cf403608
# Parent 356becdb49de5ac69f174ec8d0804882fef1ef3c
tools: libxl: translate disk device names to hd[a-d] for new qemu

We convert the first four non-SCSI disks to hd[a-d] and ignore any additional non-SCSI disks.

SCSI disks are passed through as is. qemu-xen was limited to 7 SCSI devices but upstream supports 256.

qemu-xen did all this itself internally.

Fixes "qemu: -xvda: invalid option" and allows PVHVM to work with upstream qemu.

Signed-off-by Ian Campbell <[email protected]>

diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c

tools/libxl/libxl_dm.c +14 -2
@@ -318,8 +318,27 @@ static char ** libxl__build_device_model
                 flexarray_append(dm_args, "-cdrom");
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                int dev_number = libxl__device_disk_dev_number(disks[i].vdev);
+                char *vdev;
+
+                /* Explicit sd disks are passed through as is.
+                 *
+                 * For other disks we translate devices 0..3 into
+                 * hd[a-d] and ignore the rest.
+                 */
+                if (dev_number == -1)
+                    vdev = NULL;
+                else if (strncmp(disks[i].vdev, "sd", 2) == 0)
+                    vdev = libxl__sprintf(gc, "-%s", vdev);
+                else if (dev_number < 4)
+                    vdev = libxl__sprintf(gc, "-hd%c", 'a' + dev_number);

tools/libxl/libxl_dm.c static char ** libxl__build_device_model · line 344

Quoted 10 lines
tools/libxl/libxl_dm.c +7 -0
+                else
+                    vdev = NULL;
+
+                if (vdev) {
+                    flexarray_append(dm_args, libxl__sprintf(gc, "-%s", vdev));
+                    flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                }
             }
             libxl_device_disk_destroy(&disks[i]);
         }

I don't think qemu uses -sd as the cli option to specify scsi drive and -hd[a-d] is only there for backward compatibility.
The new cli option to specify drives is -drive:

-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i] [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off] [,cache=writethrough|writeback|none|unsafe][,format=f] [,serial=s][,addr=A][,id=name][,aio=threads|native] [,readonly=on|off]

see qemu-config.c:qemu_drive_opts

#5 lore raw
Quoted 49 lines · Stefano Stabellini
Quoted 48 lines · Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301663315 -3600
# Node ID cd7f020e44b051542b82d54b8a4ef526cf403608
# Parent 356becdb49de5ac69f174ec8d0804882fef1ef3c
tools: libxl: translate disk device names to hd[a-d] for new qemu

We convert the first four non-SCSI disks to hd[a-d] and ignore any additional non-SCSI disks.

SCSI disks are passed through as is. qemu-xen was limited to 7 SCSI devices but upstream supports 256.

qemu-xen did all this itself internally.

Fixes "qemu: -xvda: invalid option" and allows PVHVM to work with upstream qemu.

Signed-off-by Ian Campbell <[email protected]>

diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c

tools/libxl/libxl_dm.c +19 -2
@@ -318,8 +318,27 @@ static char ** libxl__build_device_model
                 flexarray_append(dm_args, "-cdrom");
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                int dev_number = libxl__device_disk_dev_number(disks[i].vdev);
+                char *vdev;
+
+                /* Explicit sd disks are passed through as is.
+                 *
+                 * For other disks we translate devices 0..3 into
+                 * hd[a-d] and ignore the rest.
+                 */
+                if (dev_number == -1)
+                    vdev = NULL;
+                else if (strncmp(disks[i].vdev, "sd", 2) == 0)
+                    vdev = libxl__sprintf(gc, "-%s", vdev);
+                else if (dev_number < 4)
+                    vdev = libxl__sprintf(gc, "-hd%c", 'a' + dev_number);
+                else
+                    vdev = NULL;
+
+                if (vdev) {
+                    flexarray_append(dm_args, libxl__sprintf(gc, "-%s", vdev));
Quoted 8 lines

tools/libxl/libxl_dm.c static char ** libxl__build_device_model · line 344

Quoted 5 lines
tools/libxl/libxl_dm.c +2 -0
+                    flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                }
             }
             libxl_device_disk_destroy(&disks[i]);
         }

I don't think qemu uses -sd as the cli option to specify scsi drive and -hd[a-d] is only there for backward compatibility.

OK, that was a bug before this change too...

Quoted 7 lines

The new cli option to specify drives is -drive:

-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i] [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off] [,cache=writethrough|writeback|none|unsafe][,format=f] [,serial=s][,addr=A][,id=name][,aio=threads|native] [,readonly=on|off]

According to qemu(1) "-hd[a-d] file" maps to:
qemu -drive file=file,index=0,media=disk
qemu -drive file=file,index=1,media=disk
qemu -drive file=file,index=2,media=disk
qemu -drive file=file,index=3,media=disk

And apparently SCSI is:

qemu -drive file=file,if=scsi,bus=0,unit=6

While I'm at it I suppose -cdrom ought to become:

qemu -drive file=file,index=2,media=cdrom

I'll fix this in a separate patch though.

Ian.

author
#6 lore raw

PATCH 1/1 libxl: fixes to upstream qemu disk configuration

Quoted 1 line · On Fri, 1 Apr 2011 16:28:37 +0100, I said:

I'll fix this in a separate patch though.

Actually that doesn't make sense. Instead the following series switches to using upstream qemu's supported syntax for adding disk devices directly, instead of the compat -hda syntax and/or the non-existent -sda syntax.

It also adds an explicit format to each disk to avoid probing.

author
#7 lore raw

PATCH 1/1 libxl: explicitly set disk format in libxl__append_disk_list_of_type

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301923169 -3600
# Node ID 0191af544eee4c50fce1843099d7cd1ea98a2afd
# Parent 834eb0965ea0c90ed433d6849fbcb2c374ea40a3
libxl: explicitly set disk format in libxl__append_disk_list_of_type

Ideally we should be able to infer the format from something stashed in xenstore but this is better than letting users see garbage values.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl.c +1 -0
diff -r 834eb0965ea0 -r 0191af544eee tools/libxl/libxl.c
@@ -1787,6 +1787,7 @@ static unsigned int libxl__append_disk_l
                 pdisk->readwrite = 0;
             type = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/device-type", libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/frontend", be_path, *dir))));
             pdisk->is_cdrom = !strcmp(type, "cdrom");
+            pdisk->format = DISK_FORMAT_UNKNOWN;
         }
     }
author
#8 lore raw

PATCH 1/1 libxl: return raw disk and partition number from libxl__device_disk_dev_number

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301923169 -3600
# Node ID 8f5c77edff0b52d90cdf081891b37323df6beef3
# Parent 0191af544eee4c50fce1843099d7cd1ea98a2afd
libxl: return raw disk and partition number from libxl__device_disk_dev_number

Optional parameters, caller to follow.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl.c +4 -4
diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl.c
@@ -611,7 +611,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev, NULL, NULL)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -965,7 +965,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     }
 
     backend_type = libxl__device_disk_string_of_backend(disk->backend);
-    devid = libxl__device_disk_dev_number(disk->vdev);
+    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
                " virtual disk identifier %s", disk->vdev);

@@ -1081,7 +1081,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid, rc;
 
-    devid = libxl__device_disk_dev_number(disk->vdev);
+    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     =

@@ -1816,7 +1816,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);
tools/libxl/libxl_device.c +7 -1
diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl_device.c
@@ -200,7 +200,7 @@ static int device_virtdisk_matches(const
     return 1;
 }
 
-int libxl__device_disk_dev_number(char *virtpath)
+int libxl__device_disk_dev_number(char *virtpath, int *pdisk, int *ppartition)
 {
     int disk, partition;
     char *ep;

@@ -214,6 +214,8 @@ int libxl__device_disk_dev_number(char *
         device_virtdisk_matches(virtpath, "xvd",
                                 &disk, (1<<20)-1,
                                 &partition, 255)) {
+        if (pdisk) *pdisk = disk;
+        if (ppartition) *ppartition = partition;
         if (disk <= 15 && partition <= 15)
             return (202 << 8) | (disk << 4) | partition;
         else

@@ -228,11 +230,15 @@ int libxl__device_disk_dev_number(char *
     if (device_virtdisk_matches(virtpath, "hd",
                                 &disk, 3,
                                 &partition, 63)) {
+        if (pdisk) *pdisk = disk;
+        if (ppartition) *ppartition = partition;
         return ((disk<2 ? 3 : 22) << 8) | ((disk & 1) << 6) | partition;
     }
     if (device_virtdisk_matches(virtpath, "sd",
                                 &disk, 15,
                                 &partition, 15)) {
+        if (pdisk) *pdisk = disk;
+        if (ppartition) *ppartition = partition;
         return (8 << 8) | (disk << 4) | partition;
     }
     return -1;
tools/libxl/libxl_internal.h +1 -1
diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl_internal.h
@@ -197,7 +197,7 @@ _hidden char *libxl__device_disk_string_
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
-_hidden int libxl__device_disk_dev_number(char *virtpath);
+_hidden int libxl__device_disk_dev_number(char *virtpath, int *disk, int *partition);
 
 _hidden int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents);
author
#9 lore raw

PATCH 1/1 libxl: pass list of disks to libxl__build_device_model_args

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301923169 -3600
# Node ID 8abca96d78c1368cee94c3ac5984a92aebbb2711 # Parent 8f5c77edff0b52d90cdf081891b37323df6beef3
libxl: pass list of disks to libxl__build_device_model_args

Given that we have the information available this is preferable to picking it out of xenstore instead. We already do this for VIFs.

Only the qemu upstream version makes use of it since old qemu-xen actually parses xenstore itself.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl_dm.c +12 -16
diff -r 8f5c77edff0b -r 8abca96d78c1 tools/libxl/libxl_dm.c
@@ -40,8 +40,8 @@ static const char *libxl_tapif_script(li
 
 static char ** libxl__build_device_model_args_old(libxl__gc *gc,
                                                   libxl_device_model_info *info,
-                                                  libxl_device_nic *vifs,
-                                                  int num_vifs)
+                                                  libxl_device_disk *disks, int num_disks,
+                                                  libxl_device_nic *vifs, int num_vifs)
 {
     int i;
     flexarray_t *dm_args;

@@ -172,12 +172,11 @@ static char ** libxl__build_device_model
 
 static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                                                   libxl_device_model_info *info,
-                                                  libxl_device_nic *vifs,
-                                                  int num_vifs)
+                                                  libxl_device_disk *disks, int num_disks,
+                                                  libxl_device_nic *vifs, int num_vifs)
 {
     flexarray_t *dm_args;
-    libxl_device_disk *disks;
-    int nb, i;
+    int i;
 
     dm_args = flexarray_make(16, 1);
     if (!dm_args)

@@ -312,8 +311,7 @@ static char ** libxl__build_device_model
     flexarray_append(dm_args, libxl__sprintf(gc, "%d", info->target_ram));
 
     if (info->type == XENFV) {
-        disks = libxl_device_disk_list(libxl__gc_owner(gc), info->domid, &nb);
-        for (i; i < nb; i++) {
+        for (i; i < num_disks; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));

@@ -321,9 +319,7 @@ static char ** libxl__build_device_model
                 flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
                 flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
-            libxl_device_disk_destroy(&disks[i]);
         }
-        free(disks);
     }
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);

@@ -331,8 +327,8 @@ static char ** libxl__build_device_model
 
 static char ** libxl__build_device_model_args(libxl__gc *gc,
                                               libxl_device_model_info *info,
-                                              libxl_device_nic *vifs,
-                                              int num_vifs)
+                                              libxl_device_disk *disks, int num_disks,
+                                              libxl_device_nic *vifs, int num_vifs)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int new_qemu;

@@ -340,9 +336,9 @@ static char ** libxl__build_device_model
     new_qemu = libxl_check_device_model_version(ctx, info->device_model);
 
     if (new_qemu == 1) {
-        return libxl__build_device_model_args_new(gc, info, vifs, num_vifs);
+        return libxl__build_device_model_args_new(gc, info, disks, num_disks, vifs, num_vifs);
     } else {
-        return libxl__build_device_model_args_old(gc, info, vifs, num_vifs);
+        return libxl__build_device_model_args_old(gc, info, disks, num_disks, vifs, num_vifs);
     }
 }
 

@@ -463,7 +459,7 @@ static int libxl__create_stubdom(libxl__
     xs_transaction_t t;
     libxl__device_model_starting *dm_starting = 0;
 
-    args = libxl__build_device_model_args(gc, info, vifs, num_vifs);
+    args = libxl__build_device_model_args(gc, info, disks, num_disks, vifs, num_vifs);
     if (!args) {
         ret = ERROR_FAIL;
         goto out;

@@ -637,7 +633,7 @@ int libxl__create_device_model(libxl__gc
         goto out;
     }
 
-    args = libxl__build_device_model_args(gc, info, vifs, num_vifs);
+    args = libxl__build_device_model_args(gc, info, disks, num_disks, vifs, num_vifs);
     if (!args) {
         rc = ERROR_FAIL;
         goto out;
author
#10 lore raw

PATCH 1/1 libxl: specify disks using supported command line syntax for new qemu

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1301923169 -3600
# Node ID 4243987a85117cb2fe417e5290a0e894f035f85d
# Parent 8abca96d78c1368cee94c3ac5984a92aebbb2711
libxl: specify disks using supported command line syntax for new qemu

The -hdX syntax is only retained for compatibility reasons and the -sdX syntax doesn't even exist.

Additionally convert the first four non-SCSI disks to hd[a-d] and ignore any further non-SCSI disks (since qemu only supports 4 IDE devices).

SCSI disks are passed through as is. qemu-xen was limited to 7 SCSI devices but upstream qemu supports 256, therefore do not limit the number of disks on the libxl side.

qemu-xen did all this itself internally.

Fixes "qemu: -xvda: invalid option" and allows PVHVM to work with upstream qemu.

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl_dm.c +35 -4
diff -r 8abca96d78c1 -r 4243987a8511 tools/libxl/libxl_dm.c
@@ -175,6 +175,7 @@ static char ** libxl__build_device_model
                                                   libxl_device_disk *disks, int num_disks,
                                                   libxl_device_nic *vifs, int num_vifs)
 {
+    libxl_ctx *ctx = libxl__gc_owner(gc);
     flexarray_t *dm_args;
     int i;
 

@@ -312,13 +313,43 @@ static char ** libxl__build_device_model
 
     if (info->type == XENFV) {
         for (i; i < num_disks; i++) {
+            int disk, part;
+            int dev_number = libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
+            char *drive;
+
+            if (dev_number == -1) {
+                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "unable to determine disk number for %s", disks[i].vdev);
+                continue;
+            }
+
             if (disks[i].is_cdrom) {
-                flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                if (disks[i].format == DISK_FORMAT_EMPTY)
+                    drive = libxl__sprintf(gc, "if=ide,index=%d,media=cdrom", disk);
+                else
+                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=cdrom",
+                                           disks[i].pdev_path, disk);
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
+                if (disks[i].format == DISK_FORMAT_EMPTY)
+                    continue;
+
+                /*
+                 * Explicit sd disks are passed through as is.
+                 *
+                 * For other disks we translate devices 0..3 into
+                 * hd[a-d] and ignore the rest.
+                 */
+                if (strncmp(disks[i].vdev, "sd", 2) == 0)
+                    drive = libxl__sprintf(gc, "file=%s,if=scsi,bus=0,unit=%d",
+                                           disks[i].pdev_path, disk);
+                else if (disk < 4)
+                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=disk",
+                                           disks[i].pdev_path, disk);
+                else
+                    continue; /* Do not emulate this disk */
             }
+
+            flexarray_append(dm_args, "-drive");
+            flexarray_append(dm_args, drive);
         }
     }
     flexarray_append(dm_args, NULL);
author
#11 lore raw

PATCH 1/1 libxl: specific explicit disk image format to new qemu

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1301923242 -3600
# Node ID 5735c9124b586f95ad05b1a710c90664bb82e472 # Parent 4243987a85117cb2fe417e5290a0e894f035f85d
libxl: specific explicit disk image format to new qemu

Signed-off-by Ian Campbell <[email protected]>
tools/libxl/libxl_dm.c +27 -7
diff -r 4243987a8511 -r 5735c9124b58 tools/libxl/libxl_dm.c
@@ -170,6 +170,18 @@ static char ** libxl__build_device_model
     return (char **) flexarray_contents(dm_args);
 }
 
+static const char *qemu_disk_format_string(libxl_disk_format format)
+{
+    switch (format) {
+    case DISK_FORMAT_QCOW: return "qcow";
+    case DISK_FORMAT_QCOW2: return "qcow2";
+    case DISK_FORMAT_VHD: return "vpc";
+    case DISK_FORMAT_RAW: return "raw";
+    case DISK_FORMAT_EMPTY: return NULL;
+    default: return NULL;
+    }
+}
+
 static char ** libxl__build_device_model_args_new(libxl__gc *gc,
                                                   libxl_device_model_info *info,
                                                   libxl_device_disk *disks, int num_disks,

@@ -315,6 +327,7 @@ static char ** libxl__build_device_model
         for (i; i < num_disks; i++) {
             int disk, part;
             int dev_number = libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
+            const char *format = qemu_disk_format_string(disks[i].format);
             char *drive;
 
             if (dev_number == -1) {

@@ -326,11 +339,18 @@ static char ** libxl__build_device_model
                 if (disks[i].format == DISK_FORMAT_EMPTY)
                     drive = libxl__sprintf(gc, "if=ide,index=%d,media=cdrom", disk);
                 else
-                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=cdrom",
-                                           disks[i].pdev_path, disk);
+                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=cdrom,format=%s",
+                                           disks[i].pdev_path, disk, format);
             } else {
-                if (disks[i].format == DISK_FORMAT_EMPTY)
+                if (disks[i].format == DISK_FORMAT_EMPTY) {
+                    LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "cannot support empty disk format for %s", disks[i].vdev);
                     continue;
+                }
+
+                if (format == NULL) {
+                    LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "unable to determine disk image format %s", disks[i].vdev);
+                    continue;
+                }
 
                 /*
                  * Explicit sd disks are passed through as is.

@@ -339,11 +359,11 @@ static char ** libxl__build_device_model
                  * hd[a-d] and ignore the rest.
                  */
                 if (strncmp(disks[i].vdev, "sd", 2) == 0)
-                    drive = libxl__sprintf(gc, "file=%s,if=scsi,bus=0,unit=%d",
-                                           disks[i].pdev_path, disk);
+                    drive = libxl__sprintf(gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s",
+                                           disks[i].pdev_path, disk, format);
                 else if (disk < 4)
-                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=disk",
-                                           disks[i].pdev_path, disk);
+                    drive = libxl__sprintf(gc, "file=%s,if=ide,index=%d,media=disk,format=%s",
+                                           disks[i].pdev_path, disk, format);
                 else
                     continue; /* Do not emulate this disk */
             }
Applied
#12 lore raw

Re: [PATCH 5 of 5] libxl: specific explicit disk image format to new qemu

Ian Campbell writes ("[Xen-devel] [PATCH 5 of 5] libxl: specific explicit disk image format to new qemu"):

Quoted 1 line

libxl: specific explicit disk image format to new qemu

Applied all five, thanks.

Ian.