Libguestfs - Jun 2017 - Re: [PATCH 1/2] launch: add support for autodetection of appliance image format

On Fri, Jun 23, 2017 at 04:12:35PM +0300, Pavel Butsykin
wrote:
> This feature allows you to use different image formats for the fixed
> appliance. The raw format is used by default.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> ---
>  lib/create.c           |  5 +++--
>  lib/guestfs-internal.h |  2 ++
>  lib/launch-direct.c    |  2 ++
>  lib/launch-libvirt.c   | 15 +++++++++------
>  4 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/create.c b/lib/create.c
> index bd4c32ef7..aedfe8670 100644
> --- a/lib/create.c
> +++ b/lib/create.c
> @@ -272,7 +272,8 @@ disk_create_qcow2 (guestfs_h *g, const char
*orig_filename, int64_t size,
>       */
>      if (STRNEQ (backingformat, "raw") &&
>          STRNEQ (backingformat, "qcow2") &&
> -        STRNEQ (backingformat, "vmdk")) {
> +        STRNEQ (backingformat, "vmdk") &&
> +        STRNEQ (backingformat, AUTODETECTION_FORMAT)) {
>        error (g, _("invalid value for backingformat parameter
‘%s’"),
>               backingformat);
>        return -1;
I think this is patch is against an older version of libguestfs?
The current code has:

  if (optargs->bitmask & GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK) {
    backingformat = optargs->backingformat;
    if (!VALID_FORMAT (backingformat)) {
      error (g, _("invalid value for backingformat parameter ‘%s’"),
             backingformat);
      return -1;
    }
  }

so this hunk can just be omitted.
> @@ -321,7 +322,7 @@ disk_create_qcow2 (guestfs_h *g, const char
*orig_filename, int64_t size,
>      CLEANUP_FREE char *p = guestfs_int_qemu_escape_param (g, backingfile);
>      guestfs_int_add_sprintf (g, &optionsv,
"backing_file=%s", p);
>    }
> -  if (backingformat)
> +  if (backingformat && STRNEQ (backingformat,
AUTODETECTION_FORMAT))
>      guestfs_int_add_sprintf (g, &optionsv, "backing_fmt=%s",
backingformat);
>    if (preallocation)
>      guestfs_int_add_sprintf (g, &optionsv,
"preallocation=%s", preallocation);
backingformat is an optional parameter (controlled by
‘optargs->bitmask & GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK’), so
just don't pass it, rather than defining new API.  Then ...
> @@ -233,7 +232,7 @@ make_qcow2_overlay (guestfs_h *g, const char
*backing_drive,
>    optargs.bitmask = GUESTFS_DISK_CREATE_BACKINGFILE_BITMASK;
>    optargs.backingfile = backing_drive;
>    optargs.bitmask |= GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK;
> -  optargs.backingformat = format;
> +  optargs.backingformat = format ? format : AUTODETECTION_FORMAT;
... here, instead of passing "autodetection" (new API), you would
conditionally set the GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK flag
in the previous line.
> +#ifdef APPLIANCE_FMT_AUTO
What would define this?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top