Re: [libvirt] [PATCH v2] qemu: snapshot: remove the redundant 'if' check

2015-03-04 Thread Shanzhi Yu 


On 03/04/2015 09:12 PM, Peter Krempa wrote:
> On Sat, Feb 28, 2015 at 17:51:36 +0800, Shanzhi Yu wrote:
>> When the domain's source disk type is network, if source protocol
>> is rbd or sheepdog, the 'if().. break' will end the current case,
>> which lead to miss check the driver type is raw or qcow2. Libvirt
>> will allow to create internal snapshot for a running domain with
>> raw format disk which based on rbd storage.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179533
>> Signed-off-by: Shanzhi Yu 
>> ---
>>  src/qemu/qemu_driver.c | 5 -
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index e282464..544ed82 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -13422,11 +13422,6 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
>>active) < 0)
>>  goto cleanup;
>>  
>> -if (dom_disk->src->type == VIR_STORAGE_TYPE_NETWORK &&
>> -(dom_disk->src->protocol == 
>> VIR_STORAGE_NET_PROTOCOL_SHEEPDOG ||
>> - dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
>> -break;
>> -}
> The original intention was apparently that both protocols listed above
> support internal snapshots natively. One problem with that is that while
> they might support doing the snapshot, they don't provide place to store
> the memory data.
>
> So .. ACK, in the current implementation of internal snapshots we'll
> need to require that the volume is indeed a qcow2 so that metadata can
> be written.
>
> I'll push the patch shortly.

Thanks for your review.

>
> Peter

-- 
Regards
shyu

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] qemu: snapshot: remove the redundant 'if' check

2015-03-04 Thread Peter Krempa 
On Sat, Feb 28, 2015 at 17:51:36 +0800, Shanzhi Yu wrote:
> When the domain's source disk type is network, if source protocol
> is rbd or sheepdog, the 'if().. break' will end the current case,
> which lead to miss check the driver type is raw or qcow2. Libvirt
> will allow to create internal snapshot for a running domain with
> raw format disk which based on rbd storage.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179533
> Signed-off-by: Shanzhi Yu 
> ---
>  src/qemu/qemu_driver.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e282464..544ed82 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13422,11 +13422,6 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
>active) < 0)
>  goto cleanup;
>  
> -if (dom_disk->src->type == VIR_STORAGE_TYPE_NETWORK &&
> -(dom_disk->src->protocol == 
> VIR_STORAGE_NET_PROTOCOL_SHEEPDOG ||
> - dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
> -break;
> -}

The original intention was apparently that both protocols listed above
support internal snapshots natively. One problem with that is that while
they might support doing the snapshot, they don't provide place to store
the memory data.

So .. ACK, in the current implementation of internal snapshots we'll
need to require that the volume is indeed a qcow2 so that metadata can
be written.

I'll push the patch shortly.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list