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

Re: [libvirt] [PATCHv2 3/3] snapshot: rudimentary qemu support for atomic disk snapshot



On 03/19/2012 06:20 AM, Peter Krempa wrote:
> On 03/17/2012 05:33 PM, Eric Blake wrote:
>> Taking an external snapshot of just one disk is atomic, without having
>> to pause and resume the VM.  This also paves the way for later patches
>> to interact with the new qemu 'transaction' monitor command.
>>

>> +++ b/src/qemu/qemu_driver.c
>> @@ -10279,6 +10295,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>>                                                  &vm, snap, flags)<  0)
>>               goto cleanup;
>>       } else if (!virDomainObjIsActive(vm)) {
>> +        if (flags&  VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) {
>> +            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                            _("atomic snapshots of inactive domains
>> not "
>> +                              "implemented yet"));
>> +            goto cleanup;
> 
> I wonder if we shouldn't add a error code for unimplemented
> functionality to differentiate it from unsupported configurations.
> (Well, but using a configuration that uses unimplemented functionality
> makes it an unsupported configuration basically, so I don't really mind)

This exact same error message is removed in 4/3, so I don't think it
quite matters _what_ we put here.  I guess I could rebase the series to
swap 3/4 and 4/4, to avoid the churn.

> 
> 
>> +        }
>>           if (qemuDomainSnapshotCreateInactive(driver, vm, snap)<  0)
>>               goto cleanup;
>>       } else {
> 
> ACK,

Thanks for the reviews.  I may wait another day or two before actually
pushing, since I'm still in the middle of testing other snapshot-related
patches, just to make sure my tests don't turn up any other needed
last-minute changes to the ones I've posted so far.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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