Re: [PATCH 1/1] Add shrink flag to blockresize command

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

 



On 10/5/19 2:56 PM, martinsson.patrik@xxxxxxxxx wrote:
> From: patchon <martinsson.patrik@xxxxxxxxx>
> 

You probably want to fix this to match your signed-off-by name.

> These commits simply adds the '--shrink' flag to the blockresize
> command to prevent accidental shrinking of a block device. This
> behaviour is already present on the vol-resize command and it
> makes sense to mimic that behaviour.
> 

I don't have an opinion on whether the feature should be added at the
API level, I will leave that to others. CCing pkrempa

But as implemented it seems problematic to add a flag that changes the
semantics of the API, essentially requiring a new flag to get the old
behavior. I see the argument that this is a data safety issue, but maybe
apps are depending on this already? I don't know enough about the usage
to guess either way

> Only implemented in the qemu-driver atm.
> 
> Signed-off-by: Patrik Martinsson <martinsson.patrik@xxxxxxxxx>
> ---
>  src/qemu/qemu_driver.c | 15 ++++++++++++++-
>  tools/virsh-domain.c   |  7 +++++++
>  tools/virsh.pod        |  9 ++++++++-
>  3 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 95fe844c34..4e34eec796 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11265,8 +11265,10 @@ qemuDomainBlockResize(virDomainPtr dom,
>      char *device = NULL;
>      const char *nodename = NULL;
>      virDomainDiskDefPtr disk = NULL;
> +    virDomainBlockInfo info;
>  
> -    virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1);
> +    virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES |
> +                  VIR_STORAGE_VOL_RESIZE_SHRINK, -1);
>  

We shouldn't mix and match flag prefix names. Each public API should be
coupled with its own set of flag names. So you would want to call this
VIR_DOMAIN_BLOCK_RESIZE_SHRINK, and add it to
include/libvirt/libvirt-domain.h and add it to documentation in
libvirt-domain.c

- Cole

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux