Re: [libvirt] [PATCH v5 02/20] qemu-interop: ignore non-readable directories

Hi On Mon, Sep 23, 2019 at 4:35 PM Ján Tomko <jtomko(a)redhat.com&gt; wrote: > > On Mon, Sep 23, 2019 at 02:44:25PM +0400, marcandre.lureau(a)redhat.com wrote: > >From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > > > >Whether a directory is missing or is not readable doesn't make much > >difference when populating the available configs. > > > >This removes errors when firmare or vhost-user config is looked up > >from a user session libvirt if /etc/libvirt is not readable for ex. > > > >Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; > >--- > > src/qemu/qemu_interop_config.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > >diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c > >index 1f39d4b576..f3c5d2e083 100644 > >--- a/src/qemu/qemu_interop_config.c > >+++ b/src/qemu/qemu_interop_config.c > >@@ -41,8 +41,14 @@ qemuBuildFileList(virHashTablePtr files, const char *dir) > > int rc; > > int ret = -1; > > > >- if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) > >+ if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) { > >+ /* silently ignore unreadable directories */ > > This will not be silent - virDirOpenIfExists already logged an error. Right, let's drop the comment. > > You can call virFileExists upfront - which is just a wrapper to access > and then take virDirOpenIfExists errors seriously - if someone changes > the permissions in the meantime, they deserve the error. That won't work the desired way, since virFileExists() is true even when you don't have permissions to read it. > > >+ if (virLastErrorIsSystemErrno(EACCES)) { > >+ virResetLastError(); > >+ return 0; > >+ } > > return -1; > >+ } > > > > With that fixed: > Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; I see several ways forward, if any of the directory doesn't have readable permissions : 1. keep behaviour before this patch: fail entirely. This is unnecessarily strict imho 2. use this patch (remove the comment): log an error, but browse other directories 3. change the code further so that no error is logged To me, 2 is a good compromise. Does your r-b tag holds with the comment removed only?

thanks -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list