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

Michal Privoznik mprivozn at redhat.com
Thu Mar 12 16:53:49 UTC 2015


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




More information about the libvir-list mailing list