[libvirt] [PATCHv5 18/19] qemu: Add support for networked disks for block commit

[Adam Litke]

On 25/06/14 10:27 -0600, Eric Blake wrote:
>On 06/19/2014 07:59 AM, Peter Krempa wrote:
>> Now that we are able to select images from the backing chain via indexed
>> access we should also convert possible network sources to
>> qemu-compatible strings before passing them to qemu.
>
>Eventually, we'll want to use qemu's node-name functionality, also being
>added (but possibly in qemu 2.2 instead of 2.1, depends on how Jeff's
>series goes).  But for the simpler case of all files being local or all
>files being network from the same pool (that is, no mixed-mode chains),
>then this does appear to work at getting a decent name into qemu, at
>which point qemu can indeed commit to the right target.
>
>> ---
>>  src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 34 insertions(+), 5 deletions(-)
>>
>
>> +
>> +    if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE &&
>> +        topSource != disk->src) {
>
>So you are silently ignoring the flag if topSource is the active layer?
> That's okay, but reflect it in the documentation earlier in the series.
>
>> +        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                           _("this qemu doesn't support relative blockpull"));
>> +            goto endjob;
>> +        }
>> +
>> +        if (virStorageFileGetRelativeBackingPath(topSource, baseSource,
>> +                                                 &backingPath) < 0)
>> +            goto endjob;
>> +
>> +        if (!backingPath) {
>> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> +                           _("Can't keep relative backing relationship."));
>
>No '.' at end of the message.
>
>Wait - the earlier patches said that relative names would be preserved
>if possible, implying that an absolute name would still be used if a
>relative name was not possible.  But this errors out if a relative name
>was not possible.  Which is nicer to the end user, treating the flag as
>advisory or mandatory?  I'm hoping Adam can answer which he'd prefer, as
>one of the first clients of this new feature.

Thanks Eric.  If the flag was specified we need it to fail if a
relative backing path is not possible.  Otherwise the backing chain
could be rewritten such that the VM can not be started on a different
host in the future.  For us, not honoring the flag is a corruption.

For those applications that don't mind (or might handle abs paths
differently than relative ones, they could retry the operation without
the flag.  Perhaps we'll want a specific error code for this scenario
to make it easy to handle?

-- 
Adam Litke