I think in the situation this problem popped up qemu-img info didn't return at 
all. In fact the configured timeout probably aborted execution of run_command 
and therefore if we would have had the warning this would have been an obvious 
error and easy to debug, but please correct me if I'm wrong?

Anyway I'll add an errfunc to run_command to set both vars to undef in case of 
any error if you are ok with it? I don't think that we should return zero in 
any case other than when it is parsed correctly, because this has serious 
impact on user data.

> Thomas Lamprecht <t.lampre...@proxmox.com> hat am 9. September 2019 15:52 
> geschrieben:
> 
>  
> On 09.09.19 12:39, Tim Marx wrote:
> > This check ensures that disks aren't unintentionally shrunken, if the
> > size is zero due to an underlying problem.
> > ---
> > * fix indentation
> > 
> >  PVE/API2/Qemu.pm | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index 245de80..e6f3cce 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -3511,6 +3511,8 @@ __PACKAGE__->register_method({
> >         PVE::Storage::activate_volumes($storecfg, [$volid]);
> >         my $size = PVE::Storage::volume_size_info($storecfg, $volid, 5);
> > 
> > +       die "volume $volid does not exists\n" if !$size;
> 
> Hmm, but I _can_ create a 0 size disk image:
> 
> # qemu-img create -f qcow2 test.qcow2 0
> Formatting 'test.qcow2', fmt=qcow2 size=0 cluster_size=65536 
> lazy_refcounts=off refcount_bits=16
> # qemu-img info test.qcow2
> image: test.qcow2
> file format: qcow2
> virtual size: 0 (0 bytes)
> disk size: 192K
> cluster_size: 65536
> Format specific information:
>     compat: 1.1
>     lazy refcounts: false
>     refcount bits: 16
>     corrupt: false
> 
> 
> while a bit strange, it's possible and the single real use case I'd then
> have would be to resize it to some actual positive value.. Also ".. does
> not exists" is then not true.
> 
> How about: parse stderr in qemu.img info too, with that you should be
> able to check if a file really does not exists (`qemu-img info` does not
> exits with <> 0 in that case but prints on stderr) and if not then set
> $size and $used to undef? Then we can do a defined-ness check here...

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to