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

[Pavel Butsykin]

On 26.06.2017 00:29, Richard W.M. Jones wrote:
> 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 at 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.
>

will fix.

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

Thanks, will fix.

>> +#ifdef APPLIANCE_FMT_AUTO
> 
> What would define this?

/* Define to 1 if enabled autodetection of appliance image format. */
#define APPLIANCE_FMT_AUTO 1

and

/* #define APPLIANCE_FMT_AUTO 1 */

if the option is disabled. So made the most of the options in config.h

> Rich.
>