On Tue, Oct 15, 2019 at 10:46 PM Cole Robinson <crobinso@redhat.com> wrote:
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
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@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
--
Best regards,
-----------------------------------
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
