Xen devel - Apr 2012 - [PATCH 1/1] libxl: use qemu-xen with PV guests by default

qemu-xen offers better disk performances than qemu-xen-traditional
because it supports Linux native AIO: use it for PV guests if it is
available.

Changes in v2:
- check for the existence of the qemu-xen binary before setting qemu-xen
as the default device model for PV guests.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_create.c |   29 ++++++++++++++++++++++++-----
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e63c7bd..b71524e 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -71,9 +71,30 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         b_info->type != LIBXL_DOMAIN_TYPE_PV)
         return ERROR_INVAL;
 
-    if (!b_info->device_model_version)
-        b_info->device_model_version -           
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+    libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
+
+    if (!b_info->device_model_version) {
+        if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
+            b_info->device_model_version +               
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+        else {
+            const char *dm;
+            struct stat buf;
+            int rc;
+
+            b_info->device_model_version +               
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+            dm = libxl__domain_device_model(gc, b_info);
+            rc = stat(dm, &buf);
+            /* qemu-xen unavailable, use qemu-xen-traditional */
+            if (rc != 0) {
+                LIBXL__LOG(CTX, XTL_VERBOSE, "setting device model to
"
+                        "qemu-xen-traditional because qemu-xen is
unavailable");
+                b_info->device_model_version +                   
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+            }
+        }
+    }
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         if (!b_info->u.hvm.bios)
@@ -99,8 +120,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         }
     }
 
-    libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
-
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
         b_info->device_model_version !            
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
-- 
1.7.2.5

Stefano Stabellini writes ("[PATCH 1/1] libxl: use qemu-xen with PV guests
by default"):
> qemu-xen offers better disk performances than qemu-xen-traditional
> because it supports Linux native AIO: use it for PV guests if it is
> available.
...
> +            rc = stat(dm, &buf);
> +            /* qemu-xen unavailable, use qemu-xen-traditional */
Firstly, why not use access(2) rather than stat(2) ?

Secondly you ignore the errno value.  errnos other than ENOENT (or
perhaps ENOTDIR) should perhaps cause us to bomb out, and in any case
the errno value should be logged.

Ian.

On Fri, 20 Apr 2012, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH 1/1] libxl: use qemu-xen with PV
guests by default"):
> > qemu-xen offers better disk performances than qemu-xen-traditional
> > because it supports Linux native AIO: use it for PV guests if it is
> > available.
> ...
> > +            rc = stat(dm, &buf);
> > +            /* qemu-xen unavailable, use qemu-xen-traditional */
> 
> Firstly, why not use access(2) rather than stat(2) ?
> 
> Secondly you ignore the errno value.  errnos other than ENOENT (or
> perhaps ENOTDIR) should perhaps cause us to bomb out, and in any case
> the errno value should be logged.
 
you have two good points there, I''ll resubmit.