Re: [libvirt] [PATCH v2 1/6] storage: refactor qemu-img command line generation

On 02/05/13 18:10, Daniel P. Berrange wrote: > On Tue, Feb 05, 2013 at 12:56:12PM +0100, Ján Tomko wrote: >> >> - switch (imgformat) { >> - case QEMU_IMG_BACKING_FORMAT_FLAG: >> - virCommandAddArgList(cmd, "-F", backingType, vol->target.path, >> - NULL); >> - virCommandAddArgFormat(cmd, "%lluK", size_arg); >> + if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { >> + if (do_encryption) >> + virBufferAddLit(&buf, ",encryption=on"); >> >> - if (do_encryption) >> - virCommandAddArg(cmd, "-e"); >> - break; >> + if (!inputvol && vol->backingStore.path) >> + virBufferAsprintf(&buf, ",backing_fmt=%s", backingType); >> + else if (preallocate) >> + virBufferAddLit(&buf, ",preallocation=metadata"); > > This looks like it would result in "-o ,preallocation=metadata" if > the do_encryption arg is false. I don't believe that will work. Yes, all the options are added with a leading comma... > >> >> - case QEMU_IMG_BACKING_FORMAT_OPTIONS: >> - virCommandAddArg(cmd, "-o"); >> - virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, >> - do_encryption ? ",encryption=on" : ""); >> - virCommandAddArg(cmd, vol->target.path); >> - virCommandAddArgFormat(cmd, "%lluK", size_arg); >> - break; >> + if (virBufferError(&buf) > 0) { >> + virReportOOMError(); >> + goto cleanup; >> + } >> >> - default: >> - VIR_INFO("Unable to set backing store format for %s with %s", >> - vol->target.path, create_tool); >> + if ((options = virBufferContentAndReset(&buf))) >> + virCommandAddArgList(cmd, "-o", &(options[1]), NULL); ...which is ignored when the options are added to the argument list.