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

Re: [libvirt] [PATCH] Make domain save work on root-squash NFS



On Fri, Feb 19, 2010 at 02:30:08AM -0500, Laine Stump wrote:
> Move *all* file operations related to creation and writing of libvirt
> header to the domain save file into a hook function that is called by
> virFileOperation. First try to call virFileOperation as root. If that
> fails with EACCESS, and (in the case of Linux) statfs says that we're
> trying to save the file on an NFS share, rerun virFileOperation,
> telling it to fork a child process and setuid to the qemu user. This
> is the only way we can successfully create a file on a root-squashed
> NFS server.
> 
> This patch (along with setting dynamic_ownership=0 in qemu.conf)
> makes qemudDomainSave work on root-squashed NFS.
[...]
> +#ifdef __linux__
> +        /* On Linux we can also verify the FS-type of the directory. */
> +        struct statfs st;
> +        char *dirpath, *p;
[...]
> +
> +        if ((p = strrchr(dirpath, '/')) == NULL) {
> +            qemuReportError(VIR_ERR_INVALID_ARG,
> +                            _("Invalid path '%s' for domain save file"),    
> +                            path);
> +            VIR_FREE(dirpath);
> +            goto endjob;
> +        }
> +
> +        if (p == dirpath)
> +            *(p+1) = '\0';
> +        else
> +            *p = '\0';

  I wasn't sure why that was needed but since the man page states

"path is the pathname of any file within the mounted file  system"

so okay we have to get back to the directory which is an existing file.

> +        if (statfs(dirpath, &st) == -1) {
> +            virReportSystemError(rc,
> +                                 _("Failed to create domain save file '%s'"
> +                                   " statfs of '%s' failed (%d)"),
> +                                 path, errno);
> +            VIR_FREE(dirpath);
> +            goto endjob;
> +        }
> +
> +        VIR_FREE(dirpath);
> +
> +        if (st.f_type != NFS_SUPER_MAGIC) {
> +            virReportSystemError(rc,
> +                                 _("Failed to create domain save file '%s' (fstype 0x%X"),
> +                                 path, st.f_type);
> +            goto endjob;
> +        }

  Okay, we really need to error out if not on NFS (I can't think of
  another kind of filesystem which would exhibit the same behaviour)

> +#endif
> +
> +        /* Retry creating the file as driver->user */
> +
> +        if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
> +                                   S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
> +                                   driver->user, driver->group,
> +                                   qemudDomainSaveFileOpHook, &hdata,
> +                                   VIR_FILE_OP_AS_UID)) != 0) {
> +            virReportSystemError(rc, _("Error from child process creating '%s'"),
> +                                 path);
> +            goto endjob;
> +        }
> +
> +        /* Since we had to setuid to create the file, and the fstype
> +           is NFS, we assume it's a root-squashing NFS share, and that
> +           the security driver stuff would have failed anyway */
> +
> +        bypassSecurityDriver = 1;
>      }
> -    fd = -1;
>  
> -    if (driver->securityDriver &&
> +
> +    if ((!bypassSecurityDriver) &&
> +        driver->securityDriver &&
>          driver->securityDriver->domainSetSavedStateLabel &&
>          driver->securityDriver->domainSetSavedStateLabel(vm, path) == -1)
>          goto endjob;
> @@ -4081,7 +4181,8 @@ static int qemudDomainSave(virDomainPtr dom,
>      if (rc < 0)
>          goto endjob;
>  
> -    if (driver->securityDriver &&
> +    if ((!bypassSecurityDriver) &&
> +        driver->securityDriver &&
>          driver->securityDriver->domainRestoreSavedStateLabel &&
>          driver->securityDriver->domainRestoreSavedStateLabel(vm, path) == -1)
>          goto endjob;
> @@ -4106,8 +4207,6 @@ endjob:
>          vm = NULL;
>  
>  cleanup:
> -    if (fd != -1)
> -        close(fd);
>      VIR_FREE(xml);
>      if (ret != 0)
>          unlink(path);

  Okay, ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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