[libvirt] [PATCH] virsh: Checking the volume capacity before uploading a new file.

[Peter Krempa]

On Mon, Jan 08, 2018 at 13:55:10 +0100, Peter Krempa wrote:
> On Sun, Jan 07, 2018 at 21:31:24 -0200, Julio Faracco wrote:
> > The current command 'vol-upload' is not checking if the volume accepts 
> > a file bigger than its capacity. It can cause an interrupt of the 
> > upload stream. This commit adds a check that fails before starting to 
> > send new file to volume if the file is bigger.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1529059
> > 
> > Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
> > ---
> >  tools/virsh-volume.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
> > index 8265a39..1359dba 100644
> > --- a/tools/virsh-volume.c
> > +++ b/tools/virsh-volume.c
> > @@ -672,6 +672,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
> >  {
> >      const char *file = NULL;
> >      virStorageVolPtr vol = NULL;
> > +    virStorageVolInfo volumeInfo;
> >      bool ret = false;
> >      int fd = -1;
> >      virStreamPtr st = NULL;
> > @@ -701,6 +702,9 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
> >      cbData.ctl = ctl;
> >      cbData.fd = fd;
> >  
> > +    if (virStorageVolGetInfo(vol, &volumeInfo) < 0)
> > +        goto cleanup;
> > +
> >      if (vshCommandOptBool(cmd, "sparse"))
> >          flags |= VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM;
> >  
> > @@ -709,6 +713,11 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
> >          goto cleanup;
> >      }
> >  
> > +    if (volumeInfo.capacity <= virFileLength(file, fd)) {
> 
> I'm not sure that checking whether it's "equal" is a great idea.
> Especially since it should exactly fit in that case.

Also due to the implementation of virFileLength:

off_t
virFileLength(const char *path, int fd)
{
    struct stat s;

    if (fd >= 0) {
        if (fstat(fd, &s) < 0)
            return -1;
    } else {
        if (stat(path, &s) < 0)
            return -1;
    }

    if (!S_ISREG(s.st_mode))
       return -1;

    return s.st_size;

The test of S_ISREG will break upload from non-regular files.

Additionaly the virsh implementation has a parameter --length (stored in
'length' variable, which is not checked in this case. If the user wishes
to upload less than the fill file it would break too.

Also it's not checking that the size + offset fit in the volume either.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180108/7e428f57/attachment-0001.sig>