Re: [PATCH] vbox: use user cache dir when screenshotting.

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

 



On 09.03.2015 16:07, Dawid Zamirski wrote:
> For VBOX it's most likely that the connection is vbox:///session and it
> runs with local non-root account. This caused permission denied when
> LOCALSTATEDIR was used to create temp file. This patch makes use of the
> virGetUserCacheDirectory to address this problem for non-root users.
> ---
>  src/vbox/vbox_common.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 55d3624..a548252 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -7254,8 +7254,10 @@ vboxDomainScreenshot(virDomainPtr dom,
>      IMachine *machine = NULL;
>      nsresult rc;
>      char *tmp;
> +    char *cacheDir;
>      int tmp_fd = -1;
>      unsigned int max_screen;
> +    uid_t uid = geteuid();
>      char *ret = NULL;
>  
>      if (!data->vboxObj)
> @@ -7288,8 +7290,18 @@ vboxDomainScreenshot(virDomainPtr dom,
>          return NULL;
>      }
>  
> -    if (virAsprintf(&tmp, "%s/cache/libvirt/vbox.screendump.XXXXXX", LOCALSTATEDIR) < 0) {
> +    if (uid != 0)
> +        cacheDir = virGetUserCacheDirectory();

If one side of 'else' has braces, so ought the other one.

> +    else {
> +        if (virAsprintf(&cacheDir, "%s/cache/libvirt", LOCALSTATEDIR) < 0) {
> +            VBOX_RELEASE(machine);
> +            return NULL;
> +        }
> +    }
> +
> +    if (cacheDir && virAsprintf(&tmp, "%s/vbox.screendump.XXXXXX", cacheDir) < 0) {

cacheDir should not be NULL here. The only way that could be is if virGetUserCacheDirectory() failed, in which case we want to throw an error anyway. So I'm squashing this in, ACKing and pushing:

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index a548252..bd71c81 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7257,7 +7257,7 @@ vboxDomainScreenshot(virDomainPtr dom,
     char *cacheDir;
     int tmp_fd = -1;
     unsigned int max_screen;
-    uid_t uid = geteuid();
+    bool privileged = geteuid() == 0;
     char *ret = NULL;
 
     if (!data->vboxObj)
@@ -7290,13 +7290,10 @@ vboxDomainScreenshot(virDomainPtr dom,
         return NULL;
     }
 
-    if (uid != 0)
-        cacheDir = virGetUserCacheDirectory();
-    else {
-        if (virAsprintf(&cacheDir, "%s/cache/libvirt", LOCALSTATEDIR) < 0) {
-            VBOX_RELEASE(machine);
-            return NULL;
-        }
+    if ((privileged && virAsprintf(&cacheDir, "%s/cache/libvirt", LOCALSTATEDIR) < 0) ||
+        (!privileged && !(cacheDir = virGetUserCacheDirectory()))) {
+        VBOX_RELEASE(machine);
+        return NULL;
     }
 
     if (cacheDir && virAsprintf(&tmp, "%s/vbox.screendump.XXXXXX", cacheDir) < 0) {

Congratulations on your first libvirt contribution!

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]