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

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

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 7b5c5a365f2a -r d0dd569bfcb0 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Thu Mar 31 11:57:29 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Thu Mar 31 12:31:29 2011 +0100
@@ -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]);
         }

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

[Ian Jackson]

Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):
> 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.

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

[Ian Campbell]

On Thu, 2011-03-31 at 19:38 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):
> > 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

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

[Ian Campbell]

On Fri, 2011-04-01 at 13:01 +0100, Ian Campbell wrote:
> On Thu, 2011-03-31 at 19:38 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("[Xen-devel] [PATCH] tools: libxl: translate xvd[a-d] to hd[a-d] for new qemu"):
> > > 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...)

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 <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri Apr 01 14:05:22 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Fri Apr 01 14:08:35 2011 +0100
@@ -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]);
         }

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

[Stefano Stabellini]

On Fri, 1 Apr 2011, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # 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 <ian.campbell@citrix.com>
> 
> diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c	Fri Apr 01 14:05:22 2011 +0100
> +++ b/tools/libxl/libxl_dm.c	Fri Apr 01 14:08:35 2011 +0100
> @@ -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]);
>          }

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

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

[Ian Campbell]

On Fri, 2011-04-01 at 16:22 +0100, Stefano Stabellini wrote:
> On Fri, 1 Apr 2011, Ian Campbell wrote:
> > # HG changeset patch
> > # User Ian Campbell <ian.campbell@citrix.com>
> > # 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 <ian.campbell@citrix.com>
> > 
> > diff -r 356becdb49de -r cd7f020e44b0 tools/libxl/libxl_dm.c
> > --- a/tools/libxl/libxl_dm.c	Fri Apr 01 14:05:22 2011 +0100
> > +++ b/tools/libxl/libxl_dm.c	Fri Apr 01 14:08:35 2011 +0100
> > @@ -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]);
> >          }
> 
> 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...

> 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.

[PATCH 0 of 5] libxl: fixes to upstream qemu disk configuration

[Ian Campbell]

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.

[PATCH 1 of 5] libxl: explicitly set disk format in libxl__append_disk_list_of_type

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 834eb0965ea0 -r 0191af544eee tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl.c	Mon Apr 04 14:19:29 2011 +0100
@@ -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;
         }
     }

[PATCH 2 of 5] libxl: return raw disk and partition number from libxl__device_disk_dev_number

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl.c	Mon Apr 04 14:19:29 2011 +0100
@@ -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);
diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl_device.c	Mon Apr 04 14:19:29 2011 +0100
@@ -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;
diff -r 0191af544eee -r 8f5c77edff0b tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Mon Apr 04 14:19:29 2011 +0100
@@ -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);

[PATCH 3 of 5] libxl: pass list of disks to libxl__build_device_model_args

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 8f5c77edff0b -r 8abca96d78c1 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Mon Apr 04 14:19:29 2011 +0100
@@ -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;

[PATCH 4 of 5] libxl: specify disks using supported command line syntax for new qemu

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>

diff -r 8abca96d78c1 -r 4243987a8511 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Mon Apr 04 14:19:29 2011 +0100
@@ -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);

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

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1301923242 -3600
# Node ID 5735c9124b586f95ad05b1a710c90664bb82e472
# Parent  4243987a85117cb2fe417e5290a0e894f035f85d
libxl: specific explicit disk image format to new qemu

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 4243987a8511 -r 5735c9124b58 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Apr 04 14:19:29 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Mon Apr 04 14:20:42 2011 +0100
@@ -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 */
             }

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

[Ian Jackson]

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

Applied all five, thanks.

Ian.