Re: [PATCH resend] libxl, hotplug/Linux: default to phy backend for raw format file, take 2

[Fabio Fantoni <fabio.fantoni@m2r.biz>]

Il 08/01/2015 14:46, Wei Liu ha scritto:
> This patch resurrects 11a63a166. The previous patch had a bug that
> wrong "physical-device" was written to xenstore causing block script
> execution fail. This patch fixes that problem.
>
> Following configurations have been tested:
>
> 1. Raw file and PV
> 2. Raw file and HVM
> 3. Block device and PV
> 4. Block device and HVM
>
> Creation / destruction / local migration all worked.
>
> Original commit message from 11a63a166:
>
> Modify libxl and hotplug script to allow raw format file to use phy
> backend.
>
> The block script now tests the path and determine the actual type of
> file (block device or regular file) then use the actual type to
> determine which branch to run.
>
> With these changes, plus the current ordering of backend preference (phy
>> qdisk > tap), we will use phy backend for raw format file by default.

Sorry for the probably stupid question, what are the pros and cons of 
default use of phy instead qdisk for raw files as domU disk?

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>   tools/hotplug/Linux/block  |   16 +++++++++-------
>   tools/libxl/libxl.c        |    6 +++---
>   tools/libxl/libxl_device.c |    2 ++
>   tools/libxl/libxl_linux.c  |    6 +++---
>   4 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/tools/hotplug/Linux/block b/tools/hotplug/Linux/block
> index da26e22..8d2ee9d 100644
> --- a/tools/hotplug/Linux/block
> +++ b/tools/hotplug/Linux/block
> @@ -206,6 +206,13 @@ and so cannot be mounted ${m2}${when}."
>   
>   
>   t=$(xenstore_read_default "$XENBUS_PATH/type" 'MISSING')
> +p=$(xenstore_read "$XENBUS_PATH/params")
> +mode=$(xenstore_read "$XENBUS_PATH/mode")
> +if [ -b "$p" ]; then
> +    truetype="phy"
> +elif [ -f "$p" ]; then
> +    truetype="file"
> +fi
>   
>   case "$command" in
>     add)
> @@ -217,16 +224,11 @@ case "$command" in
>         exit 0
>       fi
>   
> -    if [ -n "$t" ]
> -    then
> -      p=$(xenstore_read "$XENBUS_PATH/params")
> -      mode=$(xenstore_read "$XENBUS_PATH/mode")
> -    fi
>       FRONTEND_ID=$(xenstore_read "$XENBUS_PATH/frontend-id")
>       FRONTEND_UUID=$(xenstore_read_default \
>               "/local/domain/$FRONTEND_ID/vm" 'unknown')
>   
> -    case $t in
> +    case $truetype in
>         phy)
>           dev=$(expand_dev $p)
>   
> @@ -319,7 +321,7 @@ mount it read-write in a guest domain."
>       ;;
>   
>     remove)
> -    case $t in
> +    case $truetype in
>         phy)
>   	exit 0
>   	;;
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 50a8928..fae9fac 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2413,9 +2413,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
>                   if (!disk->script &&
>                       disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
>                       int major, minor;
> -                    libxl__device_physdisk_major_minor(dev, &major, &minor);
> -                    flexarray_append_pair(back, "physical-device",
> -                            libxl__sprintf(gc, "%x:%x", major, minor));
> +                    if (!libxl__device_physdisk_major_minor(dev, &major, &minor))
> +                        flexarray_append_pair(back, "physical-device",
> +                                              libxl__sprintf(gc, "%x:%x", major, minor));
>                   }
>   
>                   assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 4b51ded..0f50d04 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -332,6 +332,8 @@ int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *mi
>       struct stat buf;
>       if (stat(physpath, &buf) < 0)
>           return -1;
> +    if (!S_ISBLK(buf.st_mode))
> +        return -1;
>       *major = major(buf.st_rdev);
>       *minor = minor(buf.st_rdev);
>       return 0;
> diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
> index ea5d8c1..b51930c 100644
> --- a/tools/libxl/libxl_linux.c
> +++ b/tools/libxl/libxl_linux.c
> @@ -19,11 +19,11 @@
>    
>   int libxl__try_phy_backend(mode_t st_mode)
>   {
> -    if (!S_ISBLK(st_mode)) {
> -        return 0;
> +    if (S_ISBLK(st_mode) || S_ISREG(st_mode)) {
> +        return 1;
>       }
>   
> -    return 1;
> +    return 0;
>   }
>   
>   #define EXT_SHIFT 28


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel