Re: [libvirt] virsh blockresize syntax is inconsistent with vol-resize and somewhat dangerous
On 11/18/19 3:16 PM, Cole Robinson wrote: > On 10/25/19 4:28 AM, Patrik Martinsson wrote: >> Hi Tim, >> >> I recently stumbled on the same thing, accidentally shrinking a blockdevice. >> >> I have written a patch for virsh that will force the user to append a >> '--force' flag if shrinking is desired. >> >> The behavior is somewhat still inconsistent with the vol-resize >> command, however a bigger rewrite is needed to make both commands >> operate exactly the same, which I don't know if actually needed. >> >> Previous discussion can be found below, >> - https://www.redhat.com/archives/libvir-list/2019-October/msg00258.html >> - https://www.redhat.com/archives/libvir-list/2019-October/msg01437.html >> >> Best regards, >> Patrik >> >> >> On Thu, Oct 24, 2019 at 6:04 PM Tim Small wrote: >>> >>> Hello, >>> >>> virsh has two commands which can be used to resize block devices - >>> "blockresize" for volumes in use by and active guest, and "vol-resize" >>> for volumes which are not in use. >>> >>> The vol-resize syntax allows to specify the size as a delta (increase or >>> decrease vs. the current size), and also refuses to shrink a volume >>> unless the "--shrink" argument is also passed. >>> >>> Most other tools which can be used for block device resizing (outside of >>> libvirt) also have similar "--shrink" argument requirements when >>> reducing the size of an existing block device. e.g. ceph requires >>> "--allow-shrink" when using the "rbd resize" command. >>> >>> The lack of such a safety device makes "blockresize" a foot-gun (which I >>> recently found to great effect when I typoed the domain name to another >>> valid domain). >>> >>> It seems I am not alone in making this error e.g. >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902171 >>> >>> One possible solution would be to make a new command e.g. "domblkresize" >>> or perhaps "live-resize", which implement the "--shrink" and "--delta" >>> behaviour to make it consistent with "vol-resize" syntax, and mark the >>> "blockresize" command as deprecated in the documentation and help (so >>> that existing automation which depends on the current behaviour doesn't >>> break). >>> >>> Any thoughts? Should I open this as an RFE? >>> > > Considering there's been multiple people hitting it, I think it's > something we should fix in libvirt. Just need buy in from other devs. To > summarize: > > 'virsh blockresize' will online resize an image path for a running VM. > It does this with the qemu block_resize monitor command via the > virDomainBlockResize API. The API doesn't provide any protection against > shrinking the disk image though, which I presume is both the less common > intention of the operation, and much less often safe to do for a running > VM. And a user typo can mean data loss > > virsh vol-resize, which is storage API virStorageVolResize, is for > offline image resizing, mostly using qemu-img. It has had a SHRINK API > flag from the outset, rejecting requests to reduce the image size unless > the flag is passed. Seems like a safe pattern to follow. > > Can we change existing blockresize behavior? I think it's reasonable; > we've added flags to other APIs that are required to restore old > behavior, UNDEFINE_NVRAM for one example. > I brought this question up in this thread: https://www.redhat.com/archives/libvir-list/2019-December/msg00817.html danpb suggested making this a protection that lives in virsh only. So, change blockresize to reject shrinking, but add a --shrink option to override that behavior, and all the code lives in tools/ so the old API behavior is preserved. You can CC me on a patch and I'll review it (but I'll be offline until January) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh blockresize syntax is inconsistent with vol-resize and somewhat dangerous
On 10/25/19 4:28 AM, Patrik Martinsson wrote: > Hi Tim, > > I recently stumbled on the same thing, accidentally shrinking a blockdevice. > > I have written a patch for virsh that will force the user to append a > '--force' flag if shrinking is desired. > > The behavior is somewhat still inconsistent with the vol-resize > command, however a bigger rewrite is needed to make both commands > operate exactly the same, which I don't know if actually needed. > > Previous discussion can be found below, > - https://www.redhat.com/archives/libvir-list/2019-October/msg00258.html > - https://www.redhat.com/archives/libvir-list/2019-October/msg01437.html > > Best regards, > Patrik > > > On Thu, Oct 24, 2019 at 6:04 PM Tim Small wrote: >> >> Hello, >> >> virsh has two commands which can be used to resize block devices - >> "blockresize" for volumes in use by and active guest, and "vol-resize" >> for volumes which are not in use. >> >> The vol-resize syntax allows to specify the size as a delta (increase or >> decrease vs. the current size), and also refuses to shrink a volume >> unless the "--shrink" argument is also passed. >> >> Most other tools which can be used for block device resizing (outside of >> libvirt) also have similar "--shrink" argument requirements when >> reducing the size of an existing block device. e.g. ceph requires >> "--allow-shrink" when using the "rbd resize" command. >> >> The lack of such a safety device makes "blockresize" a foot-gun (which I >> recently found to great effect when I typoed the domain name to another >> valid domain). >> >> It seems I am not alone in making this error e.g. >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902171 >> >> One possible solution would be to make a new command e.g. "domblkresize" >> or perhaps "live-resize", which implement the "--shrink" and "--delta" >> behaviour to make it consistent with "vol-resize" syntax, and mark the >> "blockresize" command as deprecated in the documentation and help (so >> that existing automation which depends on the current behaviour doesn't >> break). >> >> Any thoughts? Should I open this as an RFE? >> Considering there's been multiple people hitting it, I think it's something we should fix in libvirt. Just need buy in from other devs. To summarize: 'virsh blockresize' will online resize an image path for a running VM. It does this with the qemu block_resize monitor command via the virDomainBlockResize API. The API doesn't provide any protection against shrinking the disk image though, which I presume is both the less common intention of the operation, and much less often safe to do for a running VM. And a user typo can mean data loss virsh vol-resize, which is storage API virStorageVolResize, is for offline image resizing, mostly using qemu-img. It has had a SHRINK API flag from the outset, rejecting requests to reduce the image size unless the flag is passed. Seems like a safe pattern to follow. Can we change existing blockresize behavior? I think it's reasonable; we've added flags to other APIs that are required to restore old behavior, UNDEFINE_NVRAM for one example. danpb, pkrempa, eblake, thoughts? Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh blockresize syntax is inconsistent with vol-resize and somewhat dangerous
Hi Tim, I recently stumbled on the same thing, accidentally shrinking a blockdevice. I have written a patch for virsh that will force the user to append a '--force' flag if shrinking is desired. The behavior is somewhat still inconsistent with the vol-resize command, however a bigger rewrite is needed to make both commands operate exactly the same, which I don't know if actually needed. Previous discussion can be found below, - https://www.redhat.com/archives/libvir-list/2019-October/msg00258.html - https://www.redhat.com/archives/libvir-list/2019-October/msg01437.html Best regards, Patrik On Thu, Oct 24, 2019 at 6:04 PM Tim Small wrote: > > Hello, > > virsh has two commands which can be used to resize block devices - > "blockresize" for volumes in use by and active guest, and "vol-resize" > for volumes which are not in use. > > The vol-resize syntax allows to specify the size as a delta (increase or > decrease vs. the current size), and also refuses to shrink a volume > unless the "--shrink" argument is also passed. > > Most other tools which can be used for block device resizing (outside of > libvirt) also have similar "--shrink" argument requirements when > reducing the size of an existing block device. e.g. ceph requires > "--allow-shrink" when using the "rbd resize" command. > > The lack of such a safety device makes "blockresize" a foot-gun (which I > recently found to great effect when I typoed the domain name to another > valid domain). > > It seems I am not alone in making this error e.g. > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902171 > > One possible solution would be to make a new command e.g. "domblkresize" > or perhaps "live-resize", which implement the "--shrink" and "--delta" > behaviour to make it consistent with "vol-resize" syntax, and mark the > "blockresize" command as deprecated in the documentation and help (so > that existing automation which depends on the current behaviour doesn't > break). > > Any thoughts? Should I open this as an RFE? > > Thanks, > > Tim. > > > -- > South East Open Source Solutions Limited > Registered in England and Wales with company number 06134732. > Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ > VAT number: 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- Best regards, Patrik Martinsson -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virsh blockresize syntax is inconsistent with vol-resize and somewhat dangerous
Hello, virsh has two commands which can be used to resize block devices - "blockresize" for volumes in use by and active guest, and "vol-resize" for volumes which are not in use. The vol-resize syntax allows to specify the size as a delta (increase or decrease vs. the current size), and also refuses to shrink a volume unless the "--shrink" argument is also passed. Most other tools which can be used for block device resizing (outside of libvirt) also have similar "--shrink" argument requirements when reducing the size of an existing block device. e.g. ceph requires "--allow-shrink" when using the "rbd resize" command. The lack of such a safety device makes "blockresize" a foot-gun (which I recently found to great effect when I typoed the domain name to another valid domain). It seems I am not alone in making this error e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902171 One possible solution would be to make a new command e.g. "domblkresize" or perhaps "live-resize", which implement the "--shrink" and "--delta" behaviour to make it consistent with "vol-resize" syntax, and mark the "blockresize" command as deprecated in the documentation and help (so that existing automation which depends on the current behaviour doesn't break). Any thoughts? Should I open this as an RFE? Thanks, Tim. -- South East Open Source Solutions Limited Registered in England and Wales with company number 06134732. Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ VAT number: 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
