[libvirt] [PATCH] qemu: Don't change ownership of file when appending to it

[Daniel Veillard]

On Tue, May 24, 2011 at 02:54:28PM +0200, Michal Privoznik wrote:
> Saving domain to previously created file changes also its ownership.
> This is certainly not what users want if some conditions are met:
> it is a regular, local file and dynamic_ownership is off.
> ---
>  src/qemu/qemu_driver.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index db299a2..4538cf3 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1963,6 +1963,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>      bool is_reg = false;
>      unsigned long long offset;
>      int fd = -1;
> +    uid_t uid = getuid();
> +    gid_t gid = getgid();
>  
>      memset(&header, 0, sizeof(header));
>      memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
> @@ -2013,6 +2015,14 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>          is_reg = true;
>      } else {
>          is_reg = !!S_ISREG(sb.st_mode);
> +        /* If the path is regular local file which exists
> +         * already and dynamic_ownership is off, we don't
> +         * want to change it's ownership, just append the data */
> +        if (is_reg && !driver->dynamicOwnership &&
> +            virStorageFileIsSharedFS(path) == 0) {
> +            uid=sb.st_uid;
> +            gid=sb.st_gid;
> +        }
>      }
>  
>      offset = sizeof(header) + header.xml_len;
> @@ -2048,7 +2058,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>      } else {
>          if ((fd = virFileOpenAs(path, O_CREAT|O_TRUNC|O_WRONLY,
>                                  S_IRUSR|S_IWUSR,
> -                                getuid(), getgid(), 0)) < 0) {
> +                                uid, gid, 0)) < 0) {
>              /* If we failed as root, and the error was permission-denied
>                 (EACCES or EPERM), assume it's on a network-connected share
>                 where root access is restricted (eg, root-squashed NFS). If the

  The explaination sounds fine, and patch seems to implement just this,

    ACK,

but maybe give a 24 hours grace period for others to review it too, as
I'm not 100% sure :-)

Daniel

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