[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 02/18] blockjob: shuffle block rebase code



On 09/04/14 16:40, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> The existing virDomainBlockRebase code rejected the combination of
>> _RELATIVE and _COPY flags, but only by accident.  It makes sense,
>> at least for the case of _SHALLOW and not _REUSE_EXT, but to
>> implement it, libvirt would have to pre-create the file with a
>> relative backing name.
>>
>> Meanwhile, the code to forward on to the block copy code is getting
>> longer, and reorganising the function to have the block pull done
>> early makes it easier to add even more block copy prep code.
>>
>> This patch should have no semantic difference other than the quality
>> of the error message on the unsupported flag combination.
>>
>> * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Reorder code,
>> and improve error message of relative copy.
>>
>> Signed-off-by: Eric Blake <eblake redhat com>
>> ---
>>  src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 32 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 239a300..177d3e4 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -15467,6 +15467,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>>                        unsigned long bandwidth, unsigned int flags)
>>  {
>>      virDomainObjPtr vm;
>> +    const char *format = NULL;
>> +    int ret = -1;
>>
>>      virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
>>                    VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
>> @@ -15477,22 +15479,37 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>>      if (!(vm = qemuDomObjFromDomain(dom)))
>>          return -1;
>>
>> -    if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0) {
>> +    if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0)
>> +        goto cleanup;
>> +
>> +    /* For normal rebase (enhanced blockpull), the common code handles
>> +     * everything, including vm cleanup. */
>> +    if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
>> +        return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth,
>> +                                      NULL, BLOCK_JOB_PULL, flags);
>> +
>> +    /* If we got here, we are doing a block copy rebase. */
>> +    if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
>> +        format = "raw";
>> +
>> +    /* XXX: If we are doing a shallow copy but not reusing an external
>> +     * file, we should attempt to pre-create the destination with a
>> +     * relative backing chain instead of qemu's default of absolute */
>> +    if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
>> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> +                       _("Relative backing during copy not supported yet"));
>> +        goto cleanup;
> 
> If you'd shuffle this more up and add (flags &
> VIR_DOMAIN_BLOCK_REBASE_COPY) you'd be able to get rid of the cleanup
> section entirely.

Okay. Now I see that you are adding other stuff with "goto cleanup" at
this place so this makes sense.

> 
>> +    }
>> +

This is then okay as-is without any change.


Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]