[libvirt] [PATCH 1/1] Add shrink flag to blockresize command

[Han Han]

On Tue, Oct 15, 2019 at 10:46 PM Cole Robinson <crobinso at redhat.com> wrote:

> On 10/5/19 2:56 PM, martinsson.patrik at gmail.com wrote:
> > From: patchon <martinsson.patrik at gmail.com>
> >
>
> You probably want to fix this to match your signed-off-by name.
>
> > These commits simply adds the '--shrink' flag to the blockresize
>
I agree that an options should be added here to avoid undesirable disk
shrinking.
Because I have broken guest OS image for several times due to the shrinking
of
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
>
> I disagree the check of disk shrink is added to API level because it
changes
the behavior of blockresize API and may cause unexpected error in uplayer
products.

It is better to add the check to codes of **virsh** to avoid undesirable
shrinking
in cmdline user. And we can use '--force' to make blockresize always.
The procedure will be:

if (new size < capacity && no --force)
    print error("Can't shrink capacity below current unless --force
specified")
else
    call blockresize

And also we can add comments to blockresize API to avoid uplayer use
undesirable disk shrinking.

> > Only implemented in the qemu-driver atm.
> >
> > Signed-off-by: Patrik Martinsson <martinsson.patrik at gmail.com>
> > ---
> >  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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>


-- 
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.

Email: hhan at redhat.com
Phone: +861065339333
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191016/1cd0587b/attachment-0001.htm>