<div dir="ltr">Hi Michal,<div><br></div><div>I've sent v3 of the patch with the curly braces removed from one line 'if' blok. Sorry for that.</div><div><br></div><div>Regards,</div><div>Michal</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 June 2015 at 16:41, Michal Privoznik <span dir="ltr"><<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 22.06.2015 12:47, Michal Dubiel wrote:<br>
> QEMU working in vhost-user mode communicates with the other end (i.e.<br>
> some virtual router application) via unix domain sockets. This requires<br>
> that permissions for the socket files are correctly written into<br>
> /etc/apparmor.d/libvirt/libvirt-UUID.files.<br>
><br>
> Signed-off-by: Michal Dubiel <<a href="mailto:md@semihalf.com">md@semihalf.com</a>><br>
> ---<br>
> Changes since v1:<br>
> - Removed unnecessary stat() call and dead 'else' block<br>
><br>
>  src/security/virt-aa-helper.c | 25 ++++++++++++-------------<br>
>  1 file changed, 12 insertions(+), 13 deletions(-)<br>
><br>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c<br>
> index 35423b5..f39932e 100644<br>
> --- a/src/security/virt-aa-helper.c<br>
> +++ b/src/security/virt-aa-helper.c<br>
> @@ -32,7 +32,6 @@<br>
>  #include <unistd.h><br>
>  #include <errno.h><br>
>  #include <sys/types.h><br>
> -#include <sys/stat.h><br>
>  #include <fcntl.h><br>
>  #include <getopt.h><br>
>  #include <sys/utsname.h><br>
> @@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size)<br>
>  static int<br>
>  valid_path(const char *path, const bool readonly)<br>
>  {<br>
> -    struct stat sb;<br>
>      int npaths, opaths;<br>
>      const char * const restricted[] = {<br>
>          "/bin/",<br>
> @@ -592,17 +590,6 @@ valid_path(const char *path, const bool readonly)<br>
><br>
>      if (!virFileExists(path)) {<br>
>          vah_warning(_("path does not exist, skipping file type checks"));<br>
> -    } else {<br>
> -        if (stat(path, &sb) == -1)<br>
> -            return -1;<br>
> -<br>
> -        switch (sb.st_mode & S_IFMT) {<br>
> -            case S_IFSOCK:<br>
> -                return 1;<br>
> -                break;<br>
> -            default:<br>
> -                break;<br>
> -        }<br>
>      }<br>
<br>
</div></div>This leaves a one line body to the if(). Therefore 'syntax-check' is<br>
sad. With that fixed I'm inclined to ACK the patch. But I'm not too<br>
familiar with AppArmor, so unless somebody else gives another ACK, I'll<br>
push this after the release.<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
>      opaths = sizeof(override)/sizeof(*(override));<br>
> @@ -1101,6 +1088,18 @@ get_files(vahControl * ctl)<br>
>          }<br>
>      }<br>
><br>
> +    for (i = 0; i < ctl->def->nnets; i++) {<br>
> +        if (ctl->def->nets[i] &&<br>
> +                ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&<br>
> +                ctl->def->nets[i]->data.vhostuser) {<br>
> +            virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;<br>
> +<br>
> +            if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw",<br>
> +                       vhu->type) != 0)<br>
> +                goto cleanup;<br>
> +        }<br>
> +    }<br>
> +<br>
>      if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) {<br>
>          for (i = 0; i < ctl->def->nnets; i++) {<br>
>              virDomainNetDefPtr net = ctl->def->nets[i];<br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">Michal<br>
</font></span></blockquote></div><br></div>