[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/4] Avoid create/unlink with block devs used for QEMU save



On Wed, Apr 21, 2010 at 10:38:28PM +0200, Daniel Veillard wrote:
> On Wed, Apr 21, 2010 at 05:56:11PM +0100, Daniel P. Berrange wrote:
> > It is possible to use block devices with domain save/restore. Upon
> > failure QEMU unlinks the path being saved to. This isn't good when
> > it is a block device !
> > 
> > * src/qemu/qemu_driver.c: Don't unlink block devices if save fails
> > ---
> >  src/qemu/qemu_driver.c |   22 ++++++++++++++++++++--
> >  1 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 91fe963..50cd982 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -4787,6 +4787,8 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
> >      int rc;
> >      virDomainEventPtr event = NULL;
> >      qemuDomainObjPrivatePtr priv;
> > +    struct stat sb;
> > +    int is_bdev = 0;
> >  
> >      memset(&header, 0, sizeof(header));
> >      memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
> > @@ -4840,6 +4842,21 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
> >      }
> >      header.xml_len = strlen(xml) + 1;
> >  
> > +    /* path might be a pre-existing block dev, in which case
> > +     * we need to skip the create step, and also avoid unlink
> > +     * in the failure case */
> > +    if (stat(path, &sb) < 0) {
> > +        if (errno != ENOENT) {
> > +            virReportSystemError(errno, _("unable to access %s"), path);
> > +            goto endjob;
> > +        } else {
> > +            is_bdev = 0;
> > +        }
> 
>   what about EACCESS can we fail stat() but still be able to append to
> the file ? That sounds unlikely though...

IMHO, if we get EACCESS we're better just reporting the error because
its not a normal situation to be in.

> 
> > +    } else {
> > +        is_bdev = S_ISBLK(sb.st_mode);
> > +    }
> > +
> > +
> >      /* Setup hook data needed by virFileOperation hook function */
> >      hdata.dom = dom;
> >      hdata.path = path;
> > @@ -4849,7 +4866,8 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
> >      /* Write header to file, followed by XML */
> >  
> >      /* First try creating the file as root */
> > -    if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
> > +    if (!is_bdev &&
> > +        (rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
> >                                 S_IRUSR|S_IWUSR,
> >                                 getuid(), getgid(),
> >                                 qemudDomainSaveFileOpHook, &hdata,
> > @@ -5014,7 +5032,7 @@ endjob:
> >  
> >  cleanup:
> >      VIR_FREE(xml);
> > -    if (ret != 0)
> > +    if (ret != 0 && !is_bdev)
> >          unlink(path);
> >      if (vm)
> >          virDomainObjUnlock(vm);
> 
> ACK,


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]