<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 15, 2019 at 10:46 PM Cole Robinson <<span class="" style="" id=":3ab.34" tabindex="-1">crobinso</span>@<span class="" style="" id=":3ab.35" tabindex="-1">redhat</span>.com> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 10/5/19 2:56 PM, <a href="mailto:martinsson.patrik@gmail.com" target="_blank">martinsson.patrik@gmail.com</a> wrote:<br> > From: patchon <<a href="mailto:martinsson.patrik@gmail.com" target="_blank">martinsson.patrik@gmail.com</a>><br> > <br> <br> You probably want to fix this to match your signed-off-by name.<br> <br> > These commits simply adds the '--shrink' flag to the blockresize<br></blockquote><div>I agree that an options should be added here to avoid undesirable disk shrinking.</div><div>Because I have broken guest OS image for several times due to the shrinking of</div><div><span class="" style="" id=":3ab.36" tabindex="-1">blockresize</span>.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > command to prevent accidental shrinking of a block device. This<br> > behaviour is already present on the vol-resize command and it<br> > makes sense to mimic that behaviour.<br> > <br> <br> I don't have an opinion on whether the feature should be added at the<br> API level, I will leave that to others. CCing pkrempa<br> <br> But as implemented it seems problematic to add a flag that changes the<br> semantics of the API, essentially requiring a new flag to get the old<br> behavior. I see the argument that this is a data safety issue, but maybe<br> apps are depending on this already? I don't know enough about the usage<br> to guess either way<br> <br></blockquote><div>I disagree the check of disk shrink is added to API level because it changes</div><div>the behavior of blockresize API and may cause unexpected error in uplayer</div><div>products.</div><div><br></div><div>It is better to add the check to codes of **virsh** to avoid undesirable shrinking</div><div>in cmdline user. And we can use '--force' to make blockresize always.</div><div>The procedure will be:</div><div><br></div><div>if (new size < capacity && no --force)</div><div> print error("Can't shrink capacity below current unless --force specified")</div><div>else</div><div> call blockresize</div><div><br></div><div>And also we can add comments to blockresize API to avoid uplayer use</div><div>undesirable disk shrinking.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > Only implemented in the qemu-driver atm.<br> > <br> > Signed-off-by: Patrik Martinsson <<a href="mailto:martinsson.patrik@gmail.com" target="_blank">martinsson.patrik@gmail.com</a>><br> > ---<br> > src/qemu/qemu_driver.c | 15 ++++++++++++++-<br> > tools/virsh-domain.c | 7 +++++++<br> > tools/virsh.pod | 9 ++++++++-<br> > 3 files changed, 29 insertions(+), 2 deletions(-)<br> > <br> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c<br> > index 95fe844c34..4e34eec796 100644<br> > --- a/src/qemu/qemu_driver.c<br> > +++ b/src/qemu/qemu_driver.c<br> > @@ -11265,8 +11265,10 @@ qemuDomainBlockResize(virDomainPtr dom,<br> > char *device = NULL;<br> > const char *nodename = NULL;<br> > virDomainDiskDefPtr disk = NULL;<br> > + virDomainBlockInfo info;<br> > <br> > - virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1);<br> > + virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES |<br> > + VIR_STORAGE_VOL_RESIZE_SHRINK, -1);<br> > <br> <br> We shouldn't mix and match flag prefix names. Each public API should be<br> coupled with its own set of flag names. So you would want to call this<br> VIR_DOMAIN_BLOCK_RESIZE_SHRINK, and add it to<br> include/libvirt/libvirt-domain.h and add it to documentation in<br> libvirt-domain.c<br> <br> - Cole<br> <br> --<br> libvir-list mailing list<br> <a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br> <a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br> </blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Best regards,</div><div dir="ltr">-----------------------------------<br></div><div dir="ltr">Han Han<br>Quality Engineer<br><span class="" style="" id=":3ab.37" tabindex="-1">Redhat</span>.<br><br>Email: <a href="mailto:hhan@redhat.com" target="_blank"><span class="" style="" id=":3ab.38" tabindex="-1">hhan</span>@<span class="" style="" id=":3ab.39" tabindex="-1">redhat</span>.com</a><br>Phone: +861065339333<br></div></div></div></div></div></div></div>