Re: [libvirt] [PATCH v3] qemu: Remove network type limitation for qemuARPGetInterfaces
At 2018-09-25 17:37:24, "Lin Ma" wrote: >Let's ignore the checking of interface type when we call the function >qemuARPGetInterfaces to get IP from host's arp table. > >Based on suggestion from Laine. >https://www.redhat.com/archives/libvir-list/2018-September/msg00684.html > >Signed-off-by: Lin Ma >--- > src/qemu/qemu_driver.c | 3 --- > 1 file changed, 3 deletions(-) > >diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >index 10d6bca186..3110e74e0e 100644 >--- a/src/qemu/qemu_driver.c >+++ b/src/qemu/qemu_driver.c >@@ -20945,9 +20945,6 @@ qemuARPGetInterfaces(virDomainObjPtr vm, > goto cleanup; > > for (i = 0; i < vm->def->nnets; i++) { >-if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK) >-continue; >- > virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr); > for (j = 0; j < table->n; j++) { > virArpTableEntry entry = table->t[j]; >-- >2.19.0 Reviewed-by: Chen Hanxiao Regards, - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Small changes to virdbus
On 9/21/18 9:02 AM, Marc Hartmayer wrote: > Marc Hartmayer (4): > virdbus: Grab a ref as long as the while loop is executed > virdbus: Unref the D-Bus connection when closing > virdbus: Report a debug message that dbus_watch_handle() has failed > virdbus: Use the mnemonic macros for dbus_bool_t values > > src/util/virdbus.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > Reviewed-by: John Ferlan (series) and pushed, John although /me wonders what happens to @info in virDBusAddWatch when the call to virEventAddHandle fails, dbus_watch_set_data(watch, NULL, NULL) is called and the code returns FALSE. I assume it's leaked, different problem though ;-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plans for next release
On 9/25/18 9:38 AM, Daniel Veillard wrote: > On Tue, Sep 25, 2018 at 09:42:20AM +0200, Michal Privoznik wrote: >> On 09/24/2018 03:42 PM, Daniel Veillard wrote: >>> that's next week ! If we want to release around Oct 1st, I would >>> suggest to enter freeze this Wed, probably in the morning europe time, >>> then plan for an RC2 friday morning and if everything goes well, have >>> the final release next monday. >>> >>> I hope that works for everybody ! >> >> Well, I introduced metadata locking in this release and as usual there >> are some follow up patches waiting for review: >> >> https://www.redhat.com/archives/libvir-list/2018-September/msg01134.html >> >> Also, Bjoern reported some issues with the feature (/me wonders if it's >> s390 specific) and he said he'll debug this week. Anyway, those patches >> and any patch that he'll write can be viewed as a bug fix and therefore >> can be merged during freeze. > > Okay, let's proceed as expected, on Friday we can assess if Monday is still > right time for release, or if needed just postpone after Monday. > I've reviewed the patches Michal posted - so that part is a fait accompli ;-) IMO: Since usage of the metadata lock logic requires some amount of setup (e.g. changing a qemu.conf value), it's easy enough to indicate that the support isn't ready. In fact, in Michal's response to my review for patch 22/23: https://www.redhat.com/archives/libvir-list/2018-September/msg00795.html essentially indicates there's more to come in selinux. So, I assume we can still call this "feature" as a work in progress. Perhaps we need a new section in news.xml or some other means to indicate we have "experimental code" with "limited support" (similar to the QEMU development preview and/or the x- prefixed capabilities). IOW: a buyer/user beware if you trip across this, then you are on your own - until of course we fix all the "known" issues. So in summary, I believe the issues raised thus far are primarily metadata locking related, so should we really hold up the release for something that isn't fully complete or listed in news.xml as a new feature? John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/4] security: Don't try to lock NULL paths
On 9/25/18 3:34 AM, Michal Privoznik wrote: > It may happen that in the list of paths/disk sources to relabel > there is a disk source. If that is the case, the path is NULL. In > that case, we shouldn't try to lock the path. It's likely a > network disk anyway and therefore there is nothing to lock. I think this needs a tweak to reference commit 6d855abc1 which only filtered if the provided @p was a directory. This adds another filter when @p is NULL such as would be the case with networked storage. NB: The storage source is only passed for DAC and not selinux. The DAC code makes a some valiant attempts at src->path if not Local too. The selinux code has lots of branches and callers which seem to validly pass a path, but I could have missed a path or some nuance. The "key" is chasing virSecurityDACChownListAppend and virSecuritySELinuxContextListAppend where the list->[n]items is populated via VIR_APPEND_ELEMENT. Expect to spend some time on the chase! You already have an R-by and I don't have anything else to provide on this particular one other than yeah, better safe than sorry and passing NULL. Although I have to imagine the stat(NULL, ) in virFileIsDir wouldn't have been pleased. John > > Signed-off-by: Michal Privoznik > --- > src/security/security_dac.c | 3 ++- > src/security/security_selinux.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 876cca0f2f..04168feb3d 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -216,7 +216,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED, > for (i = 0; i < list->nItems; i++) { > const char *p = list->items[i]->path; > > -if (virFileIsDir(p)) > +if (!p || > +virFileIsDir(p)) > continue; > > VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index 3c847d8dcb..3adbeada14 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -227,7 +227,8 @@ virSecuritySELinuxTransactionRun(pid_t pid > ATTRIBUTE_UNUSED, > for (i = 0; i < list->nItems; i++) { > const char *p = list->items[i]->path; > > -if (virFileIsDir(p)) > +if (!p || > +virFileIsDir(p)) > continue; > > VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] security: Always spawn process for transactions
On 9/21/18 5:29 AM, Michal Privoznik wrote: > There is this latent bug which can result in virtlockd killing > libvirtd. The problem is when the following steps occur: > >Parent | Child > --+ > 1) virSecurityManagerMetadataLock(path); | >This results in connection FD to virtlockd to be | >dup()-ed and saved for later use. | > | > 2) Another thread calling fork(); | >This results in the FD from step 1) to be cloned | > | > 3) virSecurityManagerMetadataUnlock(path);| >Here, the FD is closed, but the connection is | >still kept open because child process has | >duplicated FD | > | > 4) virSecurityManagerMetadataLock(differentPath); | >Again, new connection is opened, FD is dup()-ed| > | > 5)| exec() or exit() > > Step 5) results in closing the connection from 1) to be closed, > finally. However, at this point virtlockd looks at its internal > records if PID from 1) does not still own any resources. Well it > does - in step 4) it locked differentPath. This results in > virtlockd killing libvirtd. > > Note that this happens because we do some relabelling even before > fork() at which point vm->pid is still -1. When this value is > passed down to transaction code it simply runs the transaction > in parent process (=libvirtd), which means the owner of metadata > locks is the parent process. > > Signed-off-by: Michal Privoznik > > Signed-off-by: Michal Privoznik Always good to double up on the S-o-b's when there's locks and forks involved. That makes sure the fork gets one too ;-) > --- > src/security/security_dac.c | 12 ++-- > src/security/security_selinux.c | 12 ++-- > 2 files changed, 12 insertions(+), 12 deletions(-) > I read the cover, it's not simpler than the above. It also indicates that patches 1-3 don't have any bearing. So to me that says - 2 series, one the artsy/fartsy cleanup variety and the second this edge/corner condition chase based on timing and/or other interactions. My first question is - have you bisected your changes? Since this only seems to matter for metadata locking, you don't necessarily have to start too early, but I would assume that prior to commit 8b8aefb3 there is no issue. My second question is - given my analysis in patch2, was testing of this patch done with or without patch 2 and 3? Hard to review this without having my patch2 analysis rattling around in short term memory that works. At this point, I'm thinking they were in place and may have had an impact especially since that dup() call was removed, but it's mentioned during this commit message, which is really confusing. Prior to patch2/3, virNetSocketDupFD has 4 callers of which only one calls with 'false' and that's the path having problems. libxlMigrateDstReceive (using true) libxlDomainMigrationSrcPerform (using true) qemuMigrationSrcConnect (using true) virNetClientDupFD (using false) Since your testing certainly doesn't include the first 3, I'll focus on the last one which has but one caller: virLockManagerLockDaemonAcquire meaning without patches 2-3, we got a "dup()" socket back instead of changing the existing socket to F_DUPFD_CLOEXEC. I think I just talked myself in a circle, but perhaps gives you an understanding of the struggle with all the patches in one series. > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 5aea386e7c..876cca0f2f 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -561,12 +561,12 @@ virSecurityDACTransactionCommit(virSecurityManagerPtr > mgr ATTRIBUTE_UNUSED, > goto cleanup; > } > > -if ((pid == -1 && > - virSecurityDACTransactionRun(pid, list) < 0) || > -(pid != -1 && > - virProcessRunInMountNamespace(pid, > - virSecurityDACTransactionRun, > - list) < 0)) > +if (pid == -1) > +pid = getpid(); > + > +if (virProcessRunInMountNamespace(pid, > + virSecurityDACTransactionRun, > + list) < 0) Going back to commit d41c1621, it essentially states: "To differentiate whether transaction code should fork() or not the @pid argument now accepts -1 (which means do not fork)." This commit then backtracks or undoes that by using a getpid() causing every transaction to fork. The "logic" for the decision to pass pid == -1 or not is introduced
Re: [libvirt] [PATCH 3/4] virLockManagerLockDaemonAcquire: Duplicate client FD with CLOEXEC flag
On 9/21/18 5:29 AM, Michal Privoznik wrote: > There is one caller (virSecurityManagerMetadataLock) which > duplicates the connection FD and wants to have the flag set. > However, trying to set the flag after dup() is not safe as > another thread might fork() meanwhile. Therefore, switch to > duplicating with the flag set and only let callers refine this > later. > > Signed-off-by: Michal Privoznik > --- > src/locking/domain_lock.c | 18 ++ > src/locking/lock_driver_lockd.c | 2 +- > src/rpc/virnetclient.c | 9 + > src/rpc/virnetclient.h | 2 +- > 4 files changed, 21 insertions(+), 10 deletions(-) > > diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c > index 705b132457..db20fa86a3 100644 > --- a/src/locking/domain_lock.c > +++ b/src/locking/domain_lock.c > @@ -188,6 +188,24 @@ int virDomainLockProcessStart(virLockManagerPluginPtr > plugin, > ret = virLockManagerAcquire(lock, NULL, flags, > dom->def->onLockFailure, fd); > > +if (ret >= 0 && fd && *fd >= 0 && virSetInherit(*fd, true) < 0) { Not quite sure 'how' ret > 0, but I'll go with it considering other consumers perform the same check. > +int saved_errno = errno; > +virErrorPtr origerr; > + > +virErrorPreserveLast(); > + > +if (virLockManagerRelease(lock, NULL, 0) < 0) > +VIR_WARN("Unable to release acquired resourced in cleanup path"); *resource(s) > + > +virErrorRestore(); > +errno = saved_errno; > + > +virReportSystemError(errno, "%s", > + _("Cannot disable close-on-exec flag")); > + > +ret = -1; > +} > + OK so this perhaps *is* the only thing you're after. Discounting patch2, you get a dup()'d socket and you then remove the CLOEXEC from it. The rest of the patch doesn't seem to matter. Perhaps some day there is a virNetClientDupFD consumer that wants to pass @true, then they have to add back all that you removed. John > virLockManagerFree(lock); > > return ret; > diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c > index 0c672b05b1..9b1943daa6 100644 > --- a/src/locking/lock_driver_lockd.c > +++ b/src/locking/lock_driver_lockd.c > @@ -796,7 +796,7 @@ static int > virLockManagerLockDaemonAcquire(virLockManagerPtr lock, > goto cleanup; > > if (fd && > -(*fd = virNetClientDupFD(client, false)) < 0) > +(*fd = virNetClientDupFD(client)) < 0) > goto cleanup; > > if (!(flags & VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY)) { > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c > index 40ed3fde6d..6b0ddbeaad 100644 > --- a/src/rpc/virnetclient.c > +++ b/src/rpc/virnetclient.c > @@ -712,19 +712,12 @@ int virNetClientGetFD(virNetClientPtr client) > } > > > -int virNetClientDupFD(virNetClientPtr client, bool cloexec) > +int virNetClientDupFD(virNetClientPtr client) > { > int fd; > > virObjectLock(client); > - > fd = virNetSocketDupFD(client->sock); > -if (!cloexec && fd >= 0 && virSetInherit(fd, true)) { > -virReportSystemError(errno, "%s", > - _("Cannot disable close-on-exec flag")); > -VIR_FORCE_CLOSE(fd); > -} > - > virObjectUnlock(client); > return fd; > } > diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h > index 9cf32091f5..3702f7fe5a 100644 > --- a/src/rpc/virnetclient.h > +++ b/src/rpc/virnetclient.h > @@ -95,7 +95,7 @@ void virNetClientSetCloseCallback(virNetClientPtr client, >virFreeCallback ff); > > int virNetClientGetFD(virNetClientPtr client); > -int virNetClientDupFD(virNetClientPtr client, bool cloexec); > +int virNetClientDupFD(virNetClientPtr client); > > bool virNetClientHasPassFD(virNetClientPtr client); > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virNetSocket: Be more safe with fork() around virNetSocketDupFD()
On 9/21/18 5:29 AM, Michal Privoznik wrote: > If there was a caller which would dup the client FD without > CLOEXEC flag and later decided to change the flag it wouldn't be > safe to do because fork() might have had occurred meantime. blank line > Switch to the other pattern - always dup FD with the flag set and > let callers clear the flag if they need to do so. I don't see this last sentence as happening in this patch... > > Signed-off-by: Michal Privoznik > --- > src/libxl/libxl_migration.c | 4 ++-- > src/qemu/qemu_migration.c | 2 +- > src/rpc/virnetclient.c | 10 +- > src/rpc/virnetsocket.c | 7 ++- > src/rpc/virnetsocket.h | 2 +- > 5 files changed, 15 insertions(+), 10 deletions(-) > I'm with Marc on this - documenting virNetSocketDupFD is a good thing. Although it could be considered out of the scope of what you're doing, since you are changing the semantics describing the nuance that fork brings about w/r/t fd duplication in the code rather than the commit message doesn't make someone have to go back and look at the commit message to gain more insight. It also should provide the reader with the knowledge of what virSetInherit would provide to remove if so desired. Of course while you're at it, documenting virNetClientDupFD similarly would be quite beneficial. > diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c > index fc7ccb53d0..5eb8eb1203 100644 > --- a/src/libxl/libxl_migration.c > +++ b/src/libxl/libxl_migration.c > @@ -310,7 +310,7 @@ libxlMigrateDstReceive(virNetSocketPtr sock, > } > VIR_DEBUG("Accepted migration connection." >" Spawning thread to process migration data"); > -recvfd = virNetSocketDupFD(client_sock, true); > +recvfd = virNetSocketDupFD(client_sock); > virObjectUnref(client_sock); > > /* > @@ -1254,7 +1254,7 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivatePtr > driver, > goto cleanup; > } > > -sockfd = virNetSocketDupFD(sock, true); > +sockfd = virNetSocketDupFD(sock); > virObjectUnref(sock); > > if (virDomainLockProcessPause(driver->lockManager, vm, >lockState) > < 0) > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index 825a9d399b..129be0a11a 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -3303,7 +3303,7 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, > if (virNetSocketNewConnectTCP(host, port, >AF_UNSPEC, >) == 0) { > -spec->dest.fd.qemu = virNetSocketDupFD(sock, true); > +spec->dest.fd.qemu = virNetSocketDupFD(sock); > virObjectUnref(sock); > } > if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0 || > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c > index b4d8fb2187..40ed3fde6d 100644 > --- a/src/rpc/virnetclient.c > +++ b/src/rpc/virnetclient.c > @@ -715,8 +715,16 @@ int virNetClientGetFD(virNetClientPtr client) > int virNetClientDupFD(virNetClientPtr client, bool cloexec) > { > int fd; > + > virObjectLock(client); > -fd = virNetSocketDupFD(client->sock, cloexec); > + > +fd = virNetSocketDupFD(client->sock); > +if (!cloexec && fd >= 0 && virSetInherit(fd, true)) { > +virReportSystemError(errno, "%s", > + _("Cannot disable close-on-exec flag")); > +VIR_FORCE_CLOSE(fd); > +} > + Caveat: my sock knowledge of setting or duplication of the socket fd isn't that deep, especially as it relates to fork "mechanics". So prior to this change, the call to virNetSocketDupFD(fd, false) would: fd = dup(sock->fd); Or essentially create a new socket fd from the existing one copying everything it already has... With this change you have (discounting some new error paths) fd = fcntl(sock->fd, F_DUPFD_CLOEXEC, 0); fcntl(fd, F_SETFD, ~FD_CLOEXEC) which seems to be a nop and/or unnecessary to me. Beyond that if the sock->fd didn't have CLOEXEC already, then what impact does briefly changing CLOEXEC have on something that arrives while set? And of course if sock->fd had CLOEXEC and then you clear it on the 'shared' socket, that doesn't seem right either. Seems to me the dup() does the trick and then lets the caller decide what CLOEXEC semantics they want with the duplicated fd without perhaps changing the sock->fd semantics. > virObjectUnlock(client); > return fd; > } > diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c > index 55de3b2aad..27ffa23bcd 100644 > --- a/src/rpc/virnetsocket.c > +++ b/src/rpc/virnetsocket.c > @@ -1372,14 +1372,11 @@ int virNetSocketGetFD(virNetSocketPtr sock) > } > > > -int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) > +int virNetSocketDupFD(virNetSocketPtr sock) > { > int fd; > > -if (cloexec) > -fd = fcntl(sock->fd, F_DUPFD_CLOEXEC, 0); > -
Re: [libvirt] [PATCH 1/4] security: Grab a reference to virSecurityManager for transactions
On 9/21/18 5:29 AM, Michal Privoznik wrote: > This shouldn't be needed per-se. Security manager shouldn't > disappear during transactions - it's immutable. However, it > doesn't hurt to grab a reference either - transaction code uses > it after all. > > Signed-off-by: Michal Privoznik > --- > src/security/security_dac.c | 5 +++-- > src/security/security_selinux.c | 5 +++-- > 2 files changed, 6 insertions(+), 4 deletions(-) > FWIW: I agree w/ Marc's assessment. You need a patch 0.5 ;-) to add the VIR_FREE(list->items) for selinux. It should reference commit ca25026 > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 2dbaf29ff5..5aea386e7c 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -141,6 +141,7 @@ virSecurityDACChownListFree(void *opaque) > VIR_FREE(list->items[i]); > } > VIR_FREE(list->items); > +virObjectUnref(list->manager); > VIR_FREE(list); > } > > @@ -511,12 +512,12 @@ virSecurityDACTransactionStart(virSecurityManagerPtr > mgr) > if (VIR_ALLOC(list) < 0) > return -1; > > -list->manager = mgr; > +list->manager = virObjectRef(mgr); If you move ^^^ to below vvv, then I think the VIR_FREE could still apply. Realistically all that's happening by calling ListFree is the Unref of list->manager. Same for _selinux. In fact, that'd probably be the more proper order with the Ref(mgr) being done. With all that, Reviewed-by: John Ferlan John I also assume you can add the VIR_FREE(list->items) to _selinux with an appropriate commit message as a pre-patch and that too has my R-By. > > if (virThreadLocalSet(, list) < 0) { > virReportSystemError(errno, "%s", > _("Unable to set thread local variable")); > -VIR_FREE(list); > +virSecurityDACChownListFree(list); > return -1; > } > > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index 056637e4cb..31e42afee7 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -156,6 +156,7 @@ virSecuritySELinuxContextListFree(void *opaque) > for (i = 0; i < list->nItems; i++) > virSecuritySELinuxContextItemFree(list->items[i]); > > +virObjectUnref(list->manager); > VIR_FREE(list); > } > > @@ -1054,12 +1055,12 @@ > virSecuritySELinuxTransactionStart(virSecurityManagerPtr mgr) > if (VIR_ALLOC(list) < 0) > return -1; > > -list->manager = mgr; > +list->manager = virObjectRef(mgr); > > if (virThreadLocalSet(, list) < 0) { > virReportSystemError(errno, "%s", > _("Unable to set thread local variable")); > -VIR_FREE(list); > +virSecuritySELinuxContextListFree(list); > return -1; > } > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] configure: remove check for regex.h
Introduced by: commit 542039fab09bd46f419702667cd342ae8f88b498 Fully support mingw builds Made redundant by: commit ec8a2d0327ee214111bca04c39ab61a9fc247f28 regex: gnulib guarantees that we have regex support Signed-off-by: Ján Tomko --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2d2c783dc2..8e77a1c9d2 100644 --- a/configure.ac +++ b/configure.ac @@ -360,7 +360,6 @@ AC_CHECK_HEADERS([\ net/ethernet.h \ netinet/tcp.h \ pwd.h \ - regex.h \ stdarg.h \ syslog.h \ sys/mount.h \ -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] configure: sort AC_CHECK_HEADERS argument list
Signed-off-by: Ján Tomko --- configure.ac | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index f23faf8b8f..00e2de7b1f 100644 --- a/configure.ac +++ b/configure.ac @@ -353,23 +353,23 @@ AC_CHECK_FUNCS_ONCE([\ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([\ + ifaddrs.h \ + libtasn1.h \ + linux/magic.h \ + mntent.h \ + net/ethernet.h \ + netinet/tcp.h \ pwd.h \ regex.h \ - sys/un.h \ - sys/poll.h \ + stdarg.h \ syslog.h \ - mntent.h \ - net/ethernet.h \ - linux/magic.h \ - sys/un.h \ + sys/mount.h \ + sys/poll.h \ sys/syscall.h \ sys/sysctl.h \ - netinet/tcp.h \ - ifaddrs.h \ - libtasn1.h \ sys/ucred.h \ - sys/mount.h \ - stdarg.h \ + sys/un.h \ + sys/un.h \ ]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include ]]) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] configure: remove duplicit check for sys/un.h
Commit 7c08fcc4 added this one. Signed-off-by: Ján Tomko --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 00e2de7b1f..2d2c783dc2 100644 --- a/configure.ac +++ b/configure.ac @@ -369,7 +369,6 @@ AC_CHECK_HEADERS([\ sys/sysctl.h \ sys/ucred.h \ sys/un.h \ - sys/un.h \ ]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include ]]) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] configure: remove regexec and getgrnam_r checks
Introduced by: commit 635ae38979bbb2659a21dfaa57e7c762ae8d4e88 commit 1b745219c7507595d4a09c89aa6a331eaa039e31 But their HAVE_ counterparts were never used. Signed-off-by: Ján Tomko --- configure.ac | 2 -- 1 file changed, 2 deletions(-) diff --git a/configure.ac b/configure.ac index 89bb6f86fc..39600017ab 100644 --- a/configure.ac +++ b/configure.ac @@ -327,7 +327,6 @@ AC_CHECK_FUNCS_ONCE([\ fallocate \ geteuid \ getgid \ - getgrnam_r \ getmntent_r \ getpwuid_r \ getrlimit \ @@ -338,7 +337,6 @@ AC_CHECK_FUNCS_ONCE([\ posix_fallocate \ posix_memalign \ prlimit \ - regexec \ sched_getaffinity \ setgroups \ setns \ -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] configure: do not check for kill
Introduced by: commit 3c37a171a2dea8048dfee90e1be2875f30f7eb15 Add check for kill() to fix build of cgroups on win32 Made redundant by: commit 02f1fd41f60c90b636ba0e18b37d4624fe47135d cgroup macros refactoring, part 1 Signed-off-by: Ján Tomko --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 4993013daa..89bb6f86fc 100644 --- a/configure.ac +++ b/configure.ac @@ -333,7 +333,6 @@ AC_CHECK_FUNCS_ONCE([\ getrlimit \ getuid \ if_indextoname \ - kill \ mmap \ newlocale \ posix_fallocate \ -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] configure: remove check for poll.h
Introduced by: commit b38d045dea41ea1fb41e546e61388116eddb6b3c Remove use of sys/poll.h on mingw Made redundant by: commit 0c97e70b74434b4baca9bbfc19c14bf7ff2ef304 Update event loop example programs to demonstrate best practice Signed-off-by: Ján Tomko --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 8e77a1c9d2..4993013daa 100644 --- a/configure.ac +++ b/configure.ac @@ -363,7 +363,6 @@ AC_CHECK_HEADERS([\ stdarg.h \ syslog.h \ sys/mount.h \ - sys/poll.h \ sys/syscall.h \ sys/sysctl.h \ sys/ucred.h \ -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] configure: split common header and function checks
Use one line per entry, to work better with line-based git history. Signed-off-by: Ján Tomko --- configure.ac | 56 +++- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 1b5c56b1a9..f23faf8b8f 100644 --- a/configure.ac +++ b/configure.ac @@ -322,17 +322,55 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions -AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ - getmntent_r getpwuid_r getrlimit getuid if_indextoname kill mmap \ - newlocale posix_fallocate posix_memalign prlimit regexec \ - sched_getaffinity setgroups setns setrlimit symlink sysctlbyname \ - getifaddrs sched_setscheduler unshare]) +AC_CHECK_FUNCS_ONCE([\ + cfmakeraw \ + fallocate \ + geteuid \ + getgid \ + getgrnam_r \ + getmntent_r \ + getpwuid_r \ + getrlimit \ + getuid \ + if_indextoname \ + kill \ + mmap \ + newlocale \ + posix_fallocate \ + posix_memalign \ + prlimit \ + regexec \ + sched_getaffinity \ + setgroups \ + setns \ + setrlimit \ + symlink \ + sysctlbyname \ + getifaddrs \ + sched_setscheduler \ + unshare \ + ]) dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ - sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ - sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h stdarg.h]) +AC_CHECK_HEADERS([\ + pwd.h \ + regex.h \ + sys/un.h \ + sys/poll.h \ + syslog.h \ + mntent.h \ + net/ethernet.h \ + linux/magic.h \ + sys/un.h \ + sys/syscall.h \ + sys/sysctl.h \ + netinet/tcp.h \ + ifaddrs.h \ + libtasn1.h \ + sys/ucred.h \ + sys/mount.h \ + stdarg.h \ + ]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include ]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64]) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] configure: remove some unused checks
Ján Tomko (7): configure: split common header and function checks configure: sort AC_CHECK_HEADERS argument list configure: remove duplicit check for sys/un.h configure: remove check for regex.h configure: remove check for poll.h configure: do not check for kill configure: remove regexec and getgrnam_r checks configure.ac | 50 +- 1 file changed, 41 insertions(+), 9 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc_monitor: Avoid AB / BA lock race
On 09/25/2018 04:31 PM, Mark Asselstine wrote: > On Tuesday, September 25, 2018 5:20:26 AM EDT Michal Privoznik wrote: >> On 09/24/2018 05:11 PM, Mark Asselstine wrote: > > Here is my read, again I have to caveat that this code is rather new to me. > > The object poisoning in virObjectUnref() would prevent a double free as the > second call would fail VIR_OBJECT_NOTVALID(). However, you are still correct > as the mon object would potentially be destroyed and cleaned up before the > close callback is run and thus when the the close callback is eventually run > it would be operating on an old, no longer valid object. > > So good catch on your part, thanks. Yep, makes sense. I've ACKed and pushed this. Congratulations on your first libvirt contribution. Usually it is just a typo fix but you fixed a deadlock. Impressive. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] qemu: Avoid duplicate resume events and state changes
On 9/25/18 9:19 AM, Jiri Denemark wrote: > The only place where VIR_DOMAIN_EVENT_RESUMED should be generated is the > RESUME event handler to make sure we don't generate duplicate events or > state changes. In the worse case the duplicity can revert or cover > changes done by other event handlers. > > For example, after QEMU sent RESUME, BLOCK_IO_ERROR, and STOP events > we could happily mark the domain as running and report > VIR_DOMAIN_EVENT_RESUMED to registered clients. > > https://bugzilla.redhat.com/show_bug.cgi?id=1612943 > > Signed-off-by: Jiri Denemark > --- > > Notes: > Version 2: > - keep VIR_DOMAIN_EVENT_RESUMED_MIGRATED event at the end of post-copy > migration > > src/qemu/qemu_driver.c| 13 --- > src/qemu/qemu_migration.c | 49 --- > src/qemu/qemu_process.c | 10 > 3 files changed, 24 insertions(+), 48 deletions(-) > This resolves the issue I noted, so Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc_monitor: Avoid AB / BA lock race
On Tuesday, September 25, 2018 5:20:26 AM EDT Michal Privoznik wrote: > On 09/24/2018 05:11 PM, Mark Asselstine wrote: > > A deadlock situation can occur when autostarting a LXC domain 'guest' > > due to two threads attempting to take opposing locks while holding > > opposing locks (AB BA problem). Thread A takes and holds the 'vm' lock > > while attempting to take the 'client' lock, meanwhile, thread B takes > > and holds the 'client' lock while attempting to take the 'vm' lock. > > > > The potential for this can be seen as follows: > > > > Thread A: > > virLXCProcessAutostartDomain (takes vm lock) > > > > --> virLXCProcessStart > > > > --> virLXCProcessConnectMonitor > > > >--> virLXCMonitorNew > > > > --> virNetClientSetCloseCallback (wants client lock) > > > > Thread B: > > virNetClientIncomingEvent (takes client lock) > > > > --> virNetClientIOHandleInput > > > > --> virNetClientCallDispatch > > > >--> virNetClientCallDispatchMessage > > > > --> virNetClientProgramDispatch > > > > --> virLXCMonitorHandleEventInit > > > > --> virLXCProcessMonitorInitNotify (wants vm lock) > > > > Since these threads are scheduled independently and are preemptible it > > is possible for the deadlock scenario to occur where each thread locks > > their first lock but both will fail to get their second lock and just > > spin forever. You get something like: > > > > virLXCProcessAutostartDomain (takes vm lock) > > > > --> virLXCProcessStart > > > > --> virLXCProcessConnectMonitor > > > >--> virLXCMonitorNew > > > > <...> > > virNetClientIncomingEvent (takes client lock) > > > > --> virNetClientIOHandleInput > > > > --> virNetClientCallDispatch > > > >--> virNetClientCallDispatchMessage > > > > --> virNetClientProgramDispatch > > > > --> virLXCMonitorHandleEventInit > > > > --> virLXCProcessMonitorInitNotify (wants vm lock but spins) > > > > <...> > > > > --> virNetClientSetCloseCallback (wants client lock but spins) > > > > Neither thread ever gets the lock it needs to be able to continue > > while holding the lock that the other thread needs. > > > > The actual window for preemption which can cause this deadlock is > > rather small, between the calls to virNetClientProgramNew() and > > execution of virNetClientSetCloseCallback(), both in > > virLXCMonitorNew(). But it can be seen in real world use that this > > small window is enough. > > > > By moving the call to virNetClientSetCloseCallback() ahead of > > virNetClientProgramNew() we can close any possible chance of the > > deadlock taking place. There should be no other implications to the > > move since the close callback (in the unlikely event was called) will > > spin on the vm lock. The remaining work that takes place between the > > old call location of virNetClientSetCloseCallback() and the new > > location is unaffected by the move. > > > > Signed-off-by: Mark Asselstine > > --- > > > > NOTE I am not intimately familiar with the libvirt code, so although I > > am confident of the cause of the issue I am not sure this is the best > > solution. So I am really curious what people who do know this code > > think and as such I am definitely open to discussions and suggestions > > on how to rework things to properly address the issue. > > > > src/lxc/lxc_monitor.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c > > index e765c16..218f426 100644 > > --- a/src/lxc/lxc_monitor.c > > +++ b/src/lxc/lxc_monitor.c > > @@ -161,6 +161,10 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, > > > > if (virNetClientRegisterAsyncIO(mon->client) < 0) > > > > goto error; > > > > +/* avoid deadlock by making this call before assigning > > virLXCMonitorEvents */ +virNetClientSetCloseCallback(mon->client, > > virLXCMonitorEOFNotify, mon, + > > virLXCMonitorCloseFreeCallback); + > > > > if (!(mon->program = virNetClientProgramNew(VIR_LXC_MONITOR_PROGRAM, > > > > VIR_LXC_MONITOR_PROGRAM_V > > ERSION, > > virLXCMonitorEvents, > > > > @@ -176,8 +180,6 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, > > > > memcpy(>cb, cb, sizeof(mon->cb)); > > > > virObjectRef(mon); > > This ref ^^ > > > -virNetClientSetCloseCallback(mon->client, virLXCMonitorEOFNotify, > > mon, > > - virLXCMonitorCloseFreeCallback); > > > > cleanup: > > VIR_FREE(sockpath); > > isn't it needed because of this SetCloseCallback, or actually > virLXCMonitorCloseFreeCallback()? The extra reference is held around so > that virLXCMonitorEOFNotify can use it and it's then released in >
Re: [libvirt] [PATCH] vircgroupv1: add ifdef around cgroup code
On Tue, Sep 25, 2018 at 03:35:11PM +0200, Pavel Hrdina wrote: Cgroups are supported only on linux. Signed-off-by: Pavel Hrdina --- src/util/vircgroupv1.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 62a6e5c448..3d05fbd745 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -55,6 +55,8 @@ VIR_ENUM_IMPL(virCgroupV1Controller, VIR_CGROUP_CONTROLLER_LAST, "name=systemd"); +#ifdef VIR_CGROUP_SUPPORTED + /* We're looking for at least one 'cgroup' fs mount, * which is *not* a named mount. */ static bool @@ -2099,3 +2101,15 @@ virCgroupV1Register(void) { virCgroupBackendRegister(); } + +#else /* !VIR_CGROUP_SUPPORTED */ + +void +virCgroupV1Register(void) +{ +virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); +return -1; util/vircgroupv1.c:2112:5: error: void function 'virCgroupV1Register' should not return a value [-Wreturn-type] return -1; ^ ~~ 1 error generated. Also, vircgrouptest.c is only guarded by #ifdef __linux__. I don't know whether it was quietly broken on Linuxes that do not satisfy VIR_CGROUP_SUPPORTED and nobody cared, or the dependency was introduced by: commit 8b62008d2bc5442f7755e579ea754ffd5e3f9691 vircgrouptest: call virCgroupNewSelf instead virCgroupDetectMounts Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vircgroupv1: add ifdef around cgroup code
On Tue, Sep 25, 2018 at 03:35:11PM +0200, Pavel Hrdina wrote: > Cgroups are supported only on linux. > > Signed-off-by: Pavel Hrdina > --- > src/util/vircgroupv1.c | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c > index 62a6e5c448..3d05fbd745 100644 > --- a/src/util/vircgroupv1.c > +++ b/src/util/vircgroupv1.c > @@ -55,6 +55,8 @@ VIR_ENUM_IMPL(virCgroupV1Controller, > VIR_CGROUP_CONTROLLER_LAST, >"name=systemd"); > > > +#ifdef VIR_CGROUP_SUPPORTED this is defined in vircgroup.c, I don't think this works as expected in the current form, I'd suggested moving the definition of the macro itself to a header. Erik > + > /* We're looking for at least one 'cgroup' fs mount, > * which is *not* a named mount. */ > static bool > @@ -2099,3 +2101,15 @@ virCgroupV1Register(void) > { > virCgroupBackendRegister(); > } > + > +#else /* !VIR_CGROUP_SUPPORTED */ > + > +void > +virCgroupV1Register(void) > +{ > +virReportSystemError(ENOSYS, "%s", > + _("Control groups not supported on this platform")); > +return -1; > +} > + > +#endif /* !VIR_CGROUP_SUPPORTED */ > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plans for next release
On Tue, Sep 25, 2018 at 09:42:20AM +0200, Michal Privoznik wrote: > On 09/24/2018 03:42 PM, Daniel Veillard wrote: > > that's next week ! If we want to release around Oct 1st, I would > > suggest to enter freeze this Wed, probably in the morning europe time, > > then plan for an RC2 friday morning and if everything goes well, have > > the final release next monday. > > > > I hope that works for everybody ! > > Well, I introduced metadata locking in this release and as usual there > are some follow up patches waiting for review: > > https://www.redhat.com/archives/libvir-list/2018-September/msg01134.html > > Also, Bjoern reported some issues with the feature (/me wonders if it's > s390 specific) and he said he'll debug this week. Anyway, those patches > and any patch that he'll write can be viewed as a bug fix and therefore > can be merged during freeze. Okay, let's proceed as expected, on Friday we can assess if Monday is still right time for release, or if needed just postpone after Monday. thanks, Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vircgroupv1: add ifdef around cgroup code
Cgroups are supported only on linux. Signed-off-by: Pavel Hrdina --- src/util/vircgroupv1.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 62a6e5c448..3d05fbd745 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -55,6 +55,8 @@ VIR_ENUM_IMPL(virCgroupV1Controller, VIR_CGROUP_CONTROLLER_LAST, "name=systemd"); +#ifdef VIR_CGROUP_SUPPORTED + /* We're looking for at least one 'cgroup' fs mount, * which is *not* a named mount. */ static bool @@ -2099,3 +2101,15 @@ virCgroupV1Register(void) { virCgroupBackendRegister(); } + +#else /* !VIR_CGROUP_SUPPORTED */ + +void +virCgroupV1Register(void) +{ +virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); +return -1; +} + +#endif /* !VIR_CGROUP_SUPPORTED */ -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] qemu: Map running reason to resume event detail
Thanks to the previous commit the RESUME event handler knows what reason should be used when changing the domain state to VIR_DOMAIN_RUNNING, but the emitted VIR_DOMAIN_EVENT_RESUMED event still uses a generic VIR_DOMAIN_EVENT_RESUMED_UNPAUSED detail. Luckily, the event detail can be easily deduced from the running reason, which saves us from having to pass one more value to the handler. Signed-off-by: Jiri Denemark Reviewed-by: John Ferlan --- Notes: Version 2: - no change src/qemu/qemu_domain.c | 29 + src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b20d43059e..939b2a3da2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -13505,3 +13505,32 @@ qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv) { priv->nodenameindex = 0; } + + +virDomainEventResumedDetailType +qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason) +{ +switch (reason) { +case VIR_DOMAIN_RUNNING_RESTORED: +case VIR_DOMAIN_RUNNING_FROM_SNAPSHOT: +return VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT; + +case VIR_DOMAIN_RUNNING_MIGRATED: +case VIR_DOMAIN_RUNNING_MIGRATION_CANCELED: +return VIR_DOMAIN_EVENT_RESUMED_MIGRATED; + +case VIR_DOMAIN_RUNNING_POSTCOPY: +return VIR_DOMAIN_EVENT_RESUMED_POSTCOPY; + +case VIR_DOMAIN_RUNNING_UNKNOWN: +case VIR_DOMAIN_RUNNING_SAVE_CANCELED: +case VIR_DOMAIN_RUNNING_BOOTED: +case VIR_DOMAIN_RUNNING_UNPAUSED: +case VIR_DOMAIN_RUNNING_WAKEUP: +case VIR_DOMAIN_RUNNING_CRASHED: +case VIR_DOMAIN_RUNNING_LAST: +break; +} + +return VIR_DOMAIN_EVENT_RESUMED_UNPAUSED; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 461859e37f..2f8a1bf03c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1082,4 +1082,7 @@ char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv); unsigned int qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv); void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv); +virDomainEventResumedDetailType +qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f79db1d4cf..a7d71962da 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -714,6 +714,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv; virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED; +virDomainEventResumedDetailType eventDetail; virObjectLock(vm); @@ -729,14 +730,16 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto unlock; } +eventDetail = qemuDomainRunningReasonToResumeEvent(reason); VIR_DEBUG("Transitioned guest %s out of paused into resumed state, " - "reason '%s'", - vm->def->name, virDomainRunningReasonTypeToString(reason)); + "reason '%s', event detail %d", + vm->def->name, virDomainRunningReasonTypeToString(reason), + eventDetail); virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); + VIR_DOMAIN_EVENT_RESUMED, + eventDetail); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) { VIR_WARN("Unable to save status on vm %s after state change", -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/4] security: Don't try to lock NULL paths
On Tue, Sep 25, 2018 at 01:01:59PM +0200, Michal Privoznik wrote: > On 09/25/2018 12:47 PM, Erik Skultety wrote: > > On Tue, Sep 25, 2018 at 09:34:50AM +0200, Michal Privoznik wrote: > >> It may happen that in the list of paths/disk sources to relabel > >> there is a disk source. If that is the case, the path is NULL. In > > > > Is there a way to prevent to such a source would not make it into the list > > in > > the first place? > > Not at all. Disk source may consists of variety of strings. A path is > just one of the options. The other is host FQN for iscsi, rbd, ceph and > all sorts of other beasts. And yet, we might want to relabel them. Fair enough. Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/5] qemu: Report more appropriate running reasons
This patch replaces some rather generic VIR_DOMAIN_RUNNING_UNPAUSED reasons when changing domain state to running with more specific ones. All of them are done when libvirtd reconnects to an existing domain after being restarted and sees an unfinished migration or save. Signed-off-by: Jiri Denemark Reviewed-by: John Ferlan --- Notes: Version 2: - modified commit message src/qemu/qemu_process.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a471a1e839..417e47ec29 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3292,7 +3292,7 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver, VIR_DEBUG("Incoming migration finished, resuming domain %s", vm->def->name); if (qemuProcessStartCPUs(driver, vm, - VIR_DOMAIN_RUNNING_UNPAUSED, + VIR_DOMAIN_RUNNING_MIGRATED, QEMU_ASYNC_JOB_NONE) < 0) { VIR_WARN("Could not resume domain %s", vm->def->name); } @@ -3399,7 +3399,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, (reason == VIR_DOMAIN_PAUSED_MIGRATION || reason == VIR_DOMAIN_PAUSED_UNKNOWN)) { if (qemuProcessStartCPUs(driver, vm, - VIR_DOMAIN_RUNNING_UNPAUSED, + VIR_DOMAIN_RUNNING_MIGRATION_CANCELED, QEMU_ASYNC_JOB_NONE) < 0) { VIR_WARN("Could not resume domain %s", vm->def->name); } @@ -3457,7 +3457,7 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, reason == VIR_DOMAIN_PAUSED_MIGRATION)) || reason == VIR_DOMAIN_PAUSED_UNKNOWN)) { if (qemuProcessStartCPUs(driver, vm, - VIR_DOMAIN_RUNNING_UNPAUSED, + VIR_DOMAIN_RUNNING_SAVE_CANCELED, QEMU_ASYNC_JOB_NONE) < 0) { VIR_WARN("Could not resume domain '%s' after migration to file", vm->def->name); -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/5] qemu: Avoid duplicate resume events and state changes
The only place where VIR_DOMAIN_EVENT_RESUMED should be generated is the RESUME event handler to make sure we don't generate duplicate events or state changes. In the worse case the duplicity can revert or cover changes done by other event handlers. For example, after QEMU sent RESUME, BLOCK_IO_ERROR, and STOP events we could happily mark the domain as running and report VIR_DOMAIN_EVENT_RESUMED to registered clients. https://bugzilla.redhat.com/show_bug.cgi?id=1612943 Signed-off-by: Jiri Denemark --- Notes: Version 2: - keep VIR_DOMAIN_EVENT_RESUMED_MIGRATED event at the end of post-copy migration src/qemu/qemu_driver.c| 13 --- src/qemu/qemu_migration.c | 49 --- src/qemu/qemu_process.c | 10 3 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c1afbfd4e..126c783a0f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1861,7 +1861,6 @@ static int qemuDomainResume(virDomainPtr dom) virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; -virObjectEventPtr event = NULL; int state; int reason; virQEMUDriverConfigPtr cfg = NULL; @@ -1900,9 +1899,6 @@ static int qemuDomainResume(virDomainPtr dom) "%s", _("resume operation failed")); goto endjob; } -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto endjob; @@ -1913,7 +1909,6 @@ static int qemuDomainResume(virDomainPtr dom) cleanup: virDomainObjEndAPI(); -virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(cfg); return ret; } @@ -15983,7 +15978,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainDefPtr config = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; -bool was_running = false; bool was_stopped = false; qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; @@ -16174,7 +16168,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, priv = vm->privateData; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { /* Transitions 5, 6 */ -was_running = true; if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_FROM_SNAPSHOT, QEMU_ASYNC_JOB_START) < 0) @@ -16271,12 +16264,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, detail); -} else if (!was_running) { -/* Transition 8 */ -detail = VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT; -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - detail); } } break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 825a9d399b..67940330aa 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2982,14 +2982,10 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, virFreeError(orig_err); if (virDomainObjGetState(vm, ) == VIR_DOMAIN_PAUSED && -reason == VIR_DOMAIN_PAUSED_POSTCOPY) { +reason == VIR_DOMAIN_PAUSED_POSTCOPY) qemuMigrationAnyPostcopyFailed(driver, vm); -} else if (qemuMigrationSrcRestoreDomainState(driver, vm)) { -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); -virObjectEventStateQueue(driver->domainEventState, event); -} +else +qemuMigrationSrcRestoreDomainState(driver, vm); qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, priv->job.migParams, priv->job.apiFlags); @@ -4624,11 +4620,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, priv->job.migParams, priv->job.apiFlags); -if (qemuMigrationSrcRestoreDomainState(driver, vm)) { -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, -
[libvirt] [PATCH v2 1/5] qemu: Properly report VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT
VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT was defined but not used anywhere in our event generation code. This fixes qemuDomainRevertToSnapshot to properly report why the domain was resumed. Signed-off-by: Jiri Denemark Reviewed-by: John Ferlan --- Notes: Version 2: - no change src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d6bca186..0c1afbfd4e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16273,7 +16273,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, detail); } else if (!was_running) { /* Transition 8 */ -detail = VIR_DOMAIN_EVENT_RESUMED; +detail = VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, detail); -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/5] qemu: Change the way we generate VIR_DOMAIN_EVENT_RESUMED
https://bugzilla.redhat.com/show_bug.cgi?id=1612943 Version 2: - keep VIR_DOMAIN_EVENT_RESUMED_MIGRATED event at the end of post-copy migration Jiri Denemark (5): qemu: Properly report VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT qemu: Report more appropriate running reasons qemu: Pass running reason to RESUME event handler qemu: Map running reason to resume event detail qemu: Avoid duplicate resume events and state changes src/qemu/qemu_domain.c| 29 +++ src/qemu/qemu_domain.h| 7 ++ src/qemu/qemu_driver.c| 13 --- src/qemu/qemu_migration.c | 49 --- src/qemu/qemu_process.c | 46 ++-- 5 files changed, 85 insertions(+), 59 deletions(-) -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/5] qemu: Pass running reason to RESUME event handler
Whenever we get the RESUME event from QEMU, we change the state of the affected domain to VIR_DOMAIN_RUNNING with VIR_DOMAIN_RUNNING_UNPAUSED reason. This is fine if the domain is resumed unexpectedly, but when we sent "cont" to QEMU we usually have a better reason for the state change. The better reason is used in qemuProcessStartCPUs which also sets the domain state to running if qemuMonitorStartCPUs reports success. Thus we may end up with two state updates in a row, but the final reason is correct. This patch is a preparation for dropping the state change done in qemuMonitorStartCPUs for which we need to pass the actual running reason to the RESUME event handler and use it there instead of VIR_DOMAIN_RUNNING_UNPAUSED. Signed-off-by: Jiri Denemark Reviewed-by: John Ferlan --- Notes: Version 2: - no change src/qemu/qemu_domain.h | 4 src/qemu/qemu_process.c | 23 +-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ecbe2e8533..461859e37f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -366,6 +366,10 @@ struct _qemuDomainObjPrivate { /* counter for generating node names for qemu disks */ unsigned long long nodenameindex; + +/* qemuProcessStartCPUs stores the reason for starting vCPUs here for the + * RESUME event handler to use it */ +virDomainRunningReason runningReason; }; # define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 417e47ec29..f79db1d4cf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -712,21 +712,28 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = opaque; virObjectEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +qemuDomainObjPrivatePtr priv; +virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED; virObjectLock(vm); -if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { -qemuDomainObjPrivatePtr priv = vm->privateData; +priv = vm->privateData; +if (priv->runningReason != VIR_DOMAIN_RUNNING_UNKNOWN) { +reason = priv->runningReason; +priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; +} + +if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { if (priv->gotShutdown) { VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); goto unlock; } -VIR_DEBUG("Transitioned guest %s out of paused into resumed state", - vm->def->name); +VIR_DEBUG("Transitioned guest %s out of paused into resumed state, " + "reason '%s'", + vm->def->name, virDomainRunningReasonTypeToString(reason)); -virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, - VIR_DOMAIN_RUNNING_UNPAUSED); +virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); @@ -3087,6 +3094,8 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, } VIR_FREE(priv->lockState); +priv->runningReason = reason; + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto release; @@ -3104,6 +3113,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, return ret; release: +priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; if (virDomainLockProcessPause(driver->lockManager, vm, >lockState) < 0) VIR_WARN("Unable to release lease on %s", vm->def->name); VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); @@ -5987,6 +5997,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, priv->monError = false; priv->monStart = 0; priv->gotShutdown = false; +priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; VIR_DEBUG("Updating guest CPU definition"); if (qemuProcessUpdateGuestCPU(vm->def, priv->qemuCaps, caps, flags) < 0) -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: libxl: skip tests with nested_hvm
Commit 95d19cd unconditionally adjusted the tests to account for the conditional move of the nested_hvm setting location. Run the affected tests only for the new setup (witnessed by LIBXL_HAVE_BUILDINFO_NESTED_HVM). Signed-off-by: Ján Tomko --- Technically a build breaker fix, but I'd love to hear any ideas that would easily let us test both versions. Also, my app-emulation/xen-tools-4.9.2-r1 on Gentoo does not seem to have LIBXL_HAVE_BUILDINFO_NESTED_HVM in libxl.h, even though it was introduced in 4.9.0-rc7-831-g3c44f8ed44 tests/libxlxml2domconfigtest.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 0eba5814b1..6b1f9826d0 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -207,9 +207,16 @@ mymain(void) DO_TEST("cpu-shares-hvm"); DO_TEST("variable-clock-hvm"); DO_TEST("moredevs-hvm"); + +# ifdef LIBXL_HAVE_BUILDINFO_NESTED_HVM DO_TEST("vnuma-hvm"); +# endif + DO_TEST("multiple-ip"); + +# ifdef LIBXL_HAVE_BUILDINFO_NESTED_HVM DO_TEST("fullvirt-cpuid"); +# endif unlink("libxl-driver.log"); -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH auto partition NUMA guest domains v1 0/2] auto partition guests providing the host NUMA topology
On Tue, Sep 25, 2018 at 12:02:40 +0200, Wim Ten Have wrote: > From: Wim ten Have > > This patch extends the guest domain administration adding support > to automatically advertise the host NUMA node capabilities obtained > architecture under a guest by creating a vNUMA copy. I'm pretty sure someone would find this useful and such configuration is perfectly valid in libvirt. But I don't think there is a compelling reason to add some magic into the domain XML which would automatically expand to such configuration. It's basically a NUMA placement policy and libvirt generally tries to avoid including any kind of policies and rather just provide all the mechanisms and knobs which can be used by applications to implement any policy they like. > The mechanism is enabled by setting the check='numa' attribute under > the CPU 'host-passthrough' topology: > Anyway, this is definitely not the right place for such option. The 'check' attribute is described as "Since 3.2.0, an optional check attribute can be used to request a specific way of checking whether the virtual CPU matches the specification." and the new 'numa' value does not fit in there in any way. Moreover the code does the automatic NUMA placement at the moment libvirt parses the domain XML, which is not the right place since it would break migration, snapshots, and save/restore features. We have existing placement attributes for vcpu and numatune/memory elements which would have been much better place for implementing such feature. And event cpu/numa element could have been enhanced to support similar configuration. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 47/47] vircgroup: rename controllers to legacy
On Tue, Sep 25, 2018 at 12:49:46PM +0200, Ján Tomko wrote: > On Tue, Sep 18, 2018 at 05:46:08PM +0200, Pavel Hrdina wrote: > > Signed-off-by: Pavel Hrdina > > --- > > src/util/vircgroup.c | 6 +- > > src/util/vircgrouppriv.h | 2 +- > > src/util/vircgroupv1.c | 140 +++ > > tests/vircgrouptest.c| 14 ++-- > > 4 files changed, 81 insertions(+), 81 deletions(-) > > > > Reviewed-by: Ján Tomko > > Jano Thanks for review, pushed now with all the issues fixed. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
On 09/25/2018 02:03 PM, Ján Tomko wrote: > On Mon, Sep 24, 2018 at 03:30:02PM +0200, Michal Privoznik wrote: >> On 09/24/2018 03:14 PM, Ján Tomko wrote: >>> Please no. >>> >>> Despite my comments on the first spacing rule [0], this series slows >>> down the spacing check by 100 % (from 1.6s to 3.1s in my case). >> >> I don't think that syntax-check run time is our goal. > > Well that's just sad. Why is that? I spend more time trying to figure our what the code does than running syntax-check over it. Therefore in my view code cleanliness is more important. But if you provide some argument otherwise we can certainly discuss it. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
On Mon, Sep 24, 2018 at 03:30:02PM +0200, Michal Privoznik wrote: On 09/24/2018 03:14 PM, Ján Tomko wrote: Please no. Despite my comments on the first spacing rule [0], this series slows down the spacing check by 100 % (from 1.6s to 3.1s in my case). I don't think that syntax-check run time is our goal. Well that's just sad. Jano I think it's the code cleanliness. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/4] security: Don't try to lock NULL paths
On 09/25/2018 12:47 PM, Erik Skultety wrote: > On Tue, Sep 25, 2018 at 09:34:50AM +0200, Michal Privoznik wrote: >> It may happen that in the list of paths/disk sources to relabel >> there is a disk source. If that is the case, the path is NULL. In > > Is there a way to prevent to such a source would not make it into the list in > the first place? Not at all. Disk source may consists of variety of strings. A path is just one of the options. The other is host FQN for iscsi, rbd, ceph and all sorts of other beasts. And yet, we might want to relabel them. > >> that case, we shouldn't try to lock the path. It's likely a >> network disk anyway and therefore there is nothing to lock. >> >> Signed-off-by: Michal Privoznik >> --- >> src/security/security_dac.c | 3 ++- >> src/security/security_selinux.c | 3 ++- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/src/security/security_dac.c b/src/security/security_dac.c >> index 876cca0f2f..04168feb3d 100644 >> --- a/src/security/security_dac.c >> +++ b/src/security/security_dac.c >> @@ -216,7 +216,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED, >> for (i = 0; i < list->nItems; i++) { >> const char *p = list->items[i]->path; >> >> -if (virFileIsDir(p)) >> +if (!p || >> +virFileIsDir(p)) > > ^this easily fits on the same line... Yep, I just wanted to separate these conditions as they both serve different purposes. But I can move it onto a single line if desired. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 47/47] vircgroup: rename controllers to legacy
On Tue, Sep 18, 2018 at 05:46:08PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c | 6 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 140 +++ tests/vircgrouptest.c| 14 ++-- 4 files changed, 81 insertions(+), 81 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 46/47] vircgroup: rename virCgroupController into virCgroupV1Controller
On Tue, Sep 18, 2018 at 05:46:07PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgrouppriv.h | 8 src/util/vircgroupv1.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 45/47] vircgroup: extract virCgroupV1(Set|Get)CpusetCpus
On Tue, Sep 18, 2018 at 05:46:06PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 10 ++ src/util/vircgroupbackend.h | 10 ++ src/util/vircgroupv1.c | 24 3 files changed, 36 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/4] security: Don't try to lock NULL paths
On Tue, Sep 25, 2018 at 09:34:50AM +0200, Michal Privoznik wrote: > It may happen that in the list of paths/disk sources to relabel > there is a disk source. If that is the case, the path is NULL. In Is there a way to prevent to such a source would not make it into the list in the first place? > that case, we shouldn't try to lock the path. It's likely a > network disk anyway and therefore there is nothing to lock. > > Signed-off-by: Michal Privoznik > --- > src/security/security_dac.c | 3 ++- > src/security/security_selinux.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 876cca0f2f..04168feb3d 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -216,7 +216,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED, > for (i = 0; i < list->nItems; i++) { > const char *p = list->items[i]->path; > > -if (virFileIsDir(p)) > +if (!p || > +virFileIsDir(p)) ^this easily fits on the same line... Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 44/47] vircgroup: extract virCgroupV1(Set|Get)CpusetMemoryMigrate
On Tue, Sep 18, 2018 at 05:46:05PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 13 ++--- src/util/vircgroupbackend.h | 10 ++ src/util/vircgroupv1.c | 27 +++ 3 files changed, 39 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 43/47] vircgroup: extract virCgroupV1(Set|Get)CpusetMems
On Tue, Sep 18, 2018 at 05:46:04PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 10 ++ src/util/vircgroupbackend.h | 11 +++ src/util/vircgroupv1.c | 25 + 3 files changed, 38 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 42/47] vircgroup: extract virCgroupV1(Set|Get)FreezerState
On Tue, Sep 18, 2018 at 05:46:03PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 8 ++-- src/util/vircgroupbackend.h | 11 +++ src/util/vircgroupv1.c | 23 +++ 3 files changed, 36 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 41/47] vircgroup: extract virCgroupV1GetCpuacctStat
On Tue, Sep 18, 2018 at 05:46:02PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 39 +- src/util/vircgroupbackend.h | 6 + src/util/vircgroupv1.c | 47 + 3 files changed, 54 insertions(+), 38 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 40/47] vircgroup: extract virCgroupV1GetCpuacct*Usage
On Tue, Sep 18, 2018 at 05:46:01PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 7 ++- src/util/vircgroupbackend.h | 11 +++ src/util/vircgroupv1.c | 22 ++ 3 files changed, 35 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 39/47] vircgroup: extract virCgroupV1SupportsCpuBW
On Tue, Sep 18, 2018 at 05:46:00PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 13 + src/util/vircgroupbackend.h | 4 src/util/vircgroupv1.c | 19 +++ 3 files changed, 24 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 37/47] vircgroup: extract virCgroupV1(Set|Get)CpuCfsPeriod
On Tue, Sep 18, 2018 at 05:45:58PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 18 ++ src/util/vircgroupbackend.h | 10 ++ src/util/vircgroupv1.c | 32 3 files changed, 44 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 38/47] vircgroup: extract virCgroupV1(Set|Get)CpuCfsQuota
On Tue, Sep 18, 2018 at 05:45:59PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 20 +++- src/util/vircgroupbackend.h | 10 ++ src/util/vircgrouppriv.h| 5 + src/util/vircgroupv1.c | 32 4 files changed, 50 insertions(+), 17 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 36/47] vircgroup: extract virCgroupV1(Set|Get)CpuShares
On Tue, Sep 18, 2018 at 05:45:57PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 8 ++-- src/util/vircgroupbackend.h | 11 +++ src/util/vircgroupv1.c | 23 +++ 3 files changed, 36 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 35/47] vircgroup: extract virCgroupV1(Allow|Deny)AllDevices
On Tue, Sep 18, 2018 at 05:45:56PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 18 ++ src/util/vircgroupbackend.h | 9 + src/util/vircgroupv1.c | 31 +++ 3 files changed, 42 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/9] cgroup cleanups and preparation for v2
On Thu, Sep 20, 2018 at 09:41:28PM +0200, Fabiano Fidêncio wrote: > On Thu, Sep 20, 2018 at 10:54 AM, Pavel Hrdina wrote: Thanks for the review, pushed now. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 34/47] vircgroup: extract virCgroupV1(Allow|Deny)Device
On Tue, Sep 18, 2018 at 05:45:55PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 68 +++- src/util/vircgroupbackend.h | 17 + src/util/vircgroupv1.c | 69 + 3 files changed, 98 insertions(+), 56 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 33/47] vircgroup: extract virCgroupV1GetMemSwapUsage
On Tue, Sep 18, 2018 at 05:45:54PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 9 + src/util/vircgroupbackend.h | 5 + src/util/vircgroupv1.c | 16 3 files changed, 22 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 32/47] vircgroup: extract virCgroupV1(Set|Get)Memory*Limit
On Tue, Sep 18, 2018 at 05:45:53PM +0200, Pavel Hrdina wrote: They all need virCgroupV1GetMemoryUnlimitedKB() so it's easier to move them in one commit. Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 128 ++- src/util/vircgroupbackend.h | 30 +++ src/util/vircgrouppriv.h| 6 ++ src/util/vircgroupv1.c | 168 4 files changed, 211 insertions(+), 121 deletions(-) diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 672fb082c1..5e75e495c3 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -219,6 +219,30 @@ typedef int (*virCgroupGetMemoryUsageCB)(virCgroupPtr group, unsigned long *kb); +typedef int +(*virCgroupSetMemoryHardLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemoryHardLimitCB)(virCgroupPtr group, + unsigned long long *kb); + +typedef int +(*virCgroupSetMemorySoftLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemorySoftLimitCB)(virCgroupPtr group, + unsigned long long *kb); + +typedef int +(*virCgroupSetMemSwapHardLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemSwapHardLimitCB)(virCgroupPtr group, + unsigned long long *kb); + struct _virCgroupBackend { virCgroupBackendType type; There are just two different types here. Regardless: Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH auto partition NUMA guest domains v1 2/2] qemuxml2argv: add tests that exercise vNUMA auto partition topology
From: Wim ten Have Add tests to ensure that the virDomainNumaAutoconfig() routine to auto- partition vNUMA topology generates correct KVM/QEMU cmdline arguments under applicable setup. Signed-off-by: Wim ten Have --- .../cpu-host-passthrough-nonuma.args | 25 .../cpu-host-passthrough-nonuma.xml | 18 .../cpu-host-passthrough-numa.args| 29 +++ .../cpu-host-passthrough-numa.xml | 18 tests/qemuxml2argvtest.c | 2 ++ 5 files changed, 92 insertions(+) create mode 100644 tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.args create mode 100644 tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.xml create mode 100644 tests/qemuxml2argvdata/cpu-host-passthrough-numa.args create mode 100644 tests/qemuxml2argvdata/cpu-host-passthrough-numa.xml diff --git a/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.args b/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.args new file mode 100644 index ..4599cdfcc159 --- /dev/null +++ b/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=kvm,usb=off,dump-guest-core=off \ +-cpu host \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.xml b/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.xml new file mode 100644 index ..d8daa8c9a43a --- /dev/null +++ b/tests/qemuxml2argvdata/cpu-host-passthrough-nonuma.xml @@ -0,0 +1,18 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + 219100 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-x86_64 + + diff --git a/tests/qemuxml2argvdata/cpu-host-passthrough-numa.args b/tests/qemuxml2argvdata/cpu-host-passthrough-numa.args new file mode 100644 index ..5b7b357cc0fe --- /dev/null +++ b/tests/qemuxml2argvdata/cpu-host-passthrough-numa.args @@ -0,0 +1,29 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=kvm,usb=off,dump-guest-core=off \ +-cpu host \ +-m 216 \ +-smp 6,sockets=6,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0,cpus=4,mem=54 \ +-numa node,nodeid=1,cpus=1,cpus=5,mem=54 \ +-numa node,nodeid=2,cpus=2,mem=54 \ +-numa node,nodeid=3,cpus=3,mem=54 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/cpu-host-passthrough-numa.xml b/tests/qemuxml2argvdata/cpu-host-passthrough-numa.xml new file mode 100644 index ..39488521b27d --- /dev/null +++ b/tests/qemuxml2argvdata/cpu-host-passthrough-numa.xml @@ -0,0 +1,18 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + 219100 + 6 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-x86_64 + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a7cde3ed7e74..4b6436b32ac7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1745,6 +1745,8 @@ mymain(void) FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE, 0, GIC_NONE, NONE); DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM); +DO_TEST("cpu-host-passthrough-numa", QEMU_CAPS_NUMA); +DO_TEST("cpu-host-passthrough-nonuma", QEMU_CAPS_NUMA); DO_TEST_FAILURE("cpu-qemu-host-passthrough", QEMU_CAPS_KVM); qemuTestSetHostArch(driver.caps, VIR_ARCH_S390X); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH auto partition NUMA guest domains v1 1/2] domain: auto partition guests providing the host NUMA topology
From: Wim ten Have Add a mechanism to auto partition the host NUMA topology under the guest domain. This patch adds a framework to automatically partition the host into a small vNUMA subset defined by the guest XML given and description when are in effect and the hypervisor indicates per the host capabilities that a physical NUMA topology is in effect. The mechanism automatically renders the host capabilities provided NUMA architecture, evenly balances the guest reserved vcpu and memory amongst its vNUMA composed cells and have the cell allocated vcpus pinned towards the host NUMA node physical cpusets. This in such way that the host NUMA topology is still in effect under the partitioned guest vNUMA domain. Signed-off-by: Wim ten Have --- docs/formatdomain.html.in | 7 ++ docs/schemas/cputypes.rng | 1 + src/conf/cpu_conf.c | 3 +- src/conf/cpu_conf.h | 1 + src/conf/domain_conf.c| 166 ++ 5 files changed, 177 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1f12ab5b4214..ba073d952545 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1500,6 +1500,13 @@ The virtual CPU created by the hypervisor will be checked against the CPU specification and the domain will not be started unless the two CPUs match. + + numa + When the CPU mode='host-passthrough' check='numa' option +combination is set, libvirt auto partitions the guest domain +by rendering the host NUMA architecture. Here the virtual +CPUs and memory are evenly balanced across the defined NUMA +nodes. The vCPUs are also pinned to their physical CPUs. Since 0.9.10, an optional mode diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index 1f1e0e36d59b..d384d161ee7e 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -29,6 +29,7 @@ none partial full +numa diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 863413e75eaa..0d52f6aa4813 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -52,7 +52,8 @@ VIR_ENUM_IMPL(virCPUCheck, VIR_CPU_CHECK_LAST, "default", "none", "partial", - "full") + "full", + "numa") VIR_ENUM_IMPL(virCPUFallback, VIR_CPU_FALLBACK_LAST, "allow", diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 9f2e7ee2649d..f2e2f0bef3ae 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -68,6 +68,7 @@ typedef enum { VIR_CPU_CHECK_NONE, VIR_CPU_CHECK_PARTIAL, VIR_CPU_CHECK_FULL, +VIR_CPU_CHECK_NUMA, VIR_CPU_CHECK_LAST } virCPUCheck; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9911d56130a9..c2f9398cfe85 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1759,6 +1759,168 @@ virDomainDefGetVcpusTopology(const virDomainDef *def, } +/** + * virDomainNumaAutoconfig: auto partition guest vNUMA XML definitions + * taking the machine NUMA topology creating a small guest copy instance. + * @def: domain definition + * @caps: host capabilities + * + * Auto partitioning vNUMA guests is requested under XML configuration + * . Here libvirt takes the + * host NUMA topology, including maxvcpus, online vcpus, memory and + * pinning node cpu's where it renders the guest domain vNUMA topology + * building an architectural copy of the host. + * + * Returns 0 on success and -1 on error. + */ +static int +virDomainNumaAutoconfig(virDomainDefPtr def, +virCapsPtr caps) +{ +int ret = -1; + +if (caps && def->cpu && +def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH && +def->cpu->check == VIR_CPU_CHECK_NUMA) { + +size_t i, cell; +size_t nvcpus = 0; +size_t nnumaCell = 0; +unsigned long long memsizeCell = 0; +virBitmapPtr vnumask = NULL; +virCapsHostPtr host = >host; + +nnumaCell = host->nnumaCell; +if (!nnumaCell) +goto cleanup; + +/* Compute the online vcpus */ +for (i = 0; i < def->maxvcpus; i++) +if (def->vcpus[i]->online) +nvcpus++; + +if (nvcpus < nnumaCell) { +VIR_WARN("vNUMA disabled: %ld vcpus is insufficient " + "to arrange a vNUMA topology for %ld nodes.", + nvcpus, nnumaCell); +goto cleanup; +} + +/* Compute the memory size (memsizeCell) per arranged nnumaCell + */ +if ((memsizeCell = def->mem.total_memory / nnumaCell) == 0) +goto cleanup; + +/* Correct vNUMA can only be accomplished if the number of maxvcpus + * is a multiple of the number of physical nodes. If this is not + * possible we set
[libvirt] [RFC PATCH auto partition NUMA guest domains v1 0/2] auto partition guests providing the host NUMA topology
From: Wim ten Have This patch extends the guest domain administration adding support to automatically advertise the host NUMA node capabilities obtained architecture under a guest by creating a vNUMA copy. The mechanism is enabled by setting the check='numa' attribute under the CPU 'host-passthrough' topology: When enabled the mechanism automatically renders the host capabilities provided NUMA architecture, evenly balances the guest reserved vcpu and memory amongst its vNUMA composed cells and have the cell allocated vcpus pinned towards the host NUMA node physical cpusets. This in such way that the host NUMA topology is still in effect under the partitioned guest domain. Below example auto partitions the host 'lscpu' listed physical NUMA detail under a guest domain vNUMA description. [root@host ]# lscpu Architecture: x86_64 CPU op-mode(s):32-bit, 64-bit Byte Order:Little Endian CPU(s):240 On-line CPU(s) list: 0-239 Thread(s) per core:2 Core(s) per socket:15 Socket(s): 8 NUMA node(s): 8 Vendor ID: GenuineIntel CPU family:6 Model: 62 Model name:Intel(R) Xeon(R) CPU E7-8895 v2 @ 2.80GHz Stepping: 7 CPU MHz: 3449.555 CPU max MHz: 3600. CPU min MHz: 1200. BogoMIPS: 5586.28 Virtualization:VT-x L1d cache: 32K L1i cache: 32K L2 cache: 256K L3 cache: 38400K NUMA node0 CPU(s): 0-14,120-134 NUMA node1 CPU(s): 15-29,135-149 NUMA node2 CPU(s): 30-44,150-164 NUMA node3 CPU(s): 45-59,165-179 NUMA node4 CPU(s): 60-74,180-194 NUMA node5 CPU(s): 75-89,195-209 NUMA node6 CPU(s): 90-104,210-224 NUMA node7 CPU(s): 105-119,225-239 Flags: ... The guest 'anuma' without the auto partition rendering enabled reads; "" anuma 3f439f5f-1156-4d48-9491-945a2c0abc6d 67108864 67108864 16 hvm destroy restart destroy /usr/bin/qemu-system-x86_64 Enabling the auto partitioning the guest 'anuma' XML is rewritten as listed below; "" anuma 3f439f5f-1156-4d48-9491-945a2c0abc6d 67108864 67108864 16 hvm destroy restart destroy /usr/bin/qemu-system-x86_64 Finally the auto partitioned guest anuma 'lscpu' listed virtual vNUMA detail. [root@anuma ~]# lscpu Architecture:x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Thread(s) per core: 2 Core(s) per socket: 1 Socket(s): 8 NUMA node(s):8 Vendor ID: GenuineIntel CPU family: 6 Model: 62 Model name: Intel(R) Xeon(R) CPU E7-8895 v2 @ 2.80GHz Stepping:
Re: [libvirt] [PATCH 31/47] vircgroup: extract virCgroupV1GetMemoryUsage
On Tue, Sep 18, 2018 at 05:45:52PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 9 + src/util/vircgroupbackend.h | 5 + src/util/vircgroupv1.c | 16 3 files changed, 22 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 30/47] vircgroup: extract virCgroupV1GetMemoryStat
On Tue, Sep 18, 2018 at 05:45:51PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 67 +++--- src/util/vircgroupbackend.h | 10 ++ src/util/vircgroupv1.c | 71 + 3 files changed, 85 insertions(+), 63 deletions(-) @@ -246,6 +255,7 @@ struct _virCgroupBackend { virCgroupGetBlkioDeviceWriteBpsCB getBlkioDeviceWriteBps; virCgroupSetMemoryCB setMemory; +virCgroupGetMemoryStatCB getMemoryStat; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 3235ef3d2d..cfe29649fb 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1403,6 +1403,76 @@ virCgroupV1SetMemory(virCgroupPtr group, } +static int +virCgroupV1GetMemoryStat(virCgroupPtr group, + unsigned long long *cache, + unsigned long long *activeAnon, + unsigned long long *inactiveAnon, + unsigned long long *activeFile, + unsigned long long *inactiveFile, + unsigned long long *unevictable) +{ Original: -int ret = -1; -char *stat = NULL; New: +VIR_AUTOFREE(char *) stat = NULL; +char *line = NULL; +unsigned long long cacheVal = 0; +unsigned long long activeAnonVal = 0; +unsigned long long inactiveAnonVal = 0; +unsigned long long activeFileVal = 0; +unsigned long long inactiveFileVal = 0; +unsigned long long unevictableVal = 0; + With the VIR_AUTOFREE conversion separated: Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: Remove network type limitation for qemuARPGetInterfaces
Let's ignore the checking of interface type when we call the function qemuARPGetInterfaces to get IP from host's arp table. Based on suggestion from Laine. https://www.redhat.com/archives/libvir-list/2018-September/msg00684.html Signed-off-by: Lin Ma --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d6bca186..3110e74e0e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20945,9 +20945,6 @@ qemuARPGetInterfaces(virDomainObjPtr vm, goto cleanup; for (i = 0; i < vm->def->nnets; i++) { -if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK) -continue; - virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr); for (j = 0; j < table->n; j++) { virArpTableEntry entry = table->t[j]; -- 2.19.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 07/13] conf: Introduce parser, formatter for uid and fid
On Tue, 2018-09-25 at 17:16 +0800, Yi Min Zhao wrote: > > > So do you mean we use this kind of API in next version for review? > > > > Yes, please :) > > I'm not sure how big the change is. So I might take more time > to prepare the new version. So that we can't catch up with > the new release. Thanks for your kindness and comments! I think it's going to be a fairly sizeable change, but as we're getting awfully close to freeze anyway I would have advised against merging zPCI support this late in the cycle anyway, so timing-wise I'd say it probably won't change much. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc_monitor: Avoid AB / BA lock race
On 09/24/2018 05:11 PM, Mark Asselstine wrote: > A deadlock situation can occur when autostarting a LXC domain 'guest' > due to two threads attempting to take opposing locks while holding > opposing locks (AB BA problem). Thread A takes and holds the 'vm' lock > while attempting to take the 'client' lock, meanwhile, thread B takes > and holds the 'client' lock while attempting to take the 'vm' lock. > > The potential for this can be seen as follows: > > Thread A: > virLXCProcessAutostartDomain (takes vm lock) > --> virLXCProcessStart > --> virLXCProcessConnectMonitor >--> virLXCMonitorNew > --> virNetClientSetCloseCallback (wants client lock) > > Thread B: > virNetClientIncomingEvent (takes client lock) > --> virNetClientIOHandleInput > --> virNetClientCallDispatch >--> virNetClientCallDispatchMessage > --> virNetClientProgramDispatch > --> virLXCMonitorHandleEventInit > --> virLXCProcessMonitorInitNotify (wants vm lock) > > Since these threads are scheduled independently and are preemptible it > is possible for the deadlock scenario to occur where each thread locks > their first lock but both will fail to get their second lock and just > spin forever. You get something like: > > virLXCProcessAutostartDomain (takes vm lock) > --> virLXCProcessStart > --> virLXCProcessConnectMonitor >--> virLXCMonitorNew > <...> > virNetClientIncomingEvent (takes client lock) > --> virNetClientIOHandleInput > --> virNetClientCallDispatch >--> virNetClientCallDispatchMessage > --> virNetClientProgramDispatch > --> virLXCMonitorHandleEventInit > --> virLXCProcessMonitorInitNotify (wants vm lock but spins) > <...> > --> virNetClientSetCloseCallback (wants client lock but spins) > > Neither thread ever gets the lock it needs to be able to continue > while holding the lock that the other thread needs. > > The actual window for preemption which can cause this deadlock is > rather small, between the calls to virNetClientProgramNew() and > execution of virNetClientSetCloseCallback(), both in > virLXCMonitorNew(). But it can be seen in real world use that this > small window is enough. > > By moving the call to virNetClientSetCloseCallback() ahead of > virNetClientProgramNew() we can close any possible chance of the > deadlock taking place. There should be no other implications to the > move since the close callback (in the unlikely event was called) will > spin on the vm lock. The remaining work that takes place between the > old call location of virNetClientSetCloseCallback() and the new > location is unaffected by the move. > > Signed-off-by: Mark Asselstine > --- > > NOTE I am not intimately familiar with the libvirt code, so although I > am confident of the cause of the issue I am not sure this is the best > solution. So I am really curious what people who do know this code > think and as such I am definitely open to discussions and suggestions > on how to rework things to properly address the issue. > > src/lxc/lxc_monitor.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c > index e765c16..218f426 100644 > --- a/src/lxc/lxc_monitor.c > +++ b/src/lxc/lxc_monitor.c > @@ -161,6 +161,10 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, > if (virNetClientRegisterAsyncIO(mon->client) < 0) > goto error; > > +/* avoid deadlock by making this call before assigning > virLXCMonitorEvents */ > +virNetClientSetCloseCallback(mon->client, virLXCMonitorEOFNotify, mon, > + virLXCMonitorCloseFreeCallback); > + > if (!(mon->program = virNetClientProgramNew(VIR_LXC_MONITOR_PROGRAM, > > VIR_LXC_MONITOR_PROGRAM_VERSION, > virLXCMonitorEvents, > @@ -176,8 +180,6 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, > memcpy(>cb, cb, sizeof(mon->cb)); > > virObjectRef(mon); This ref ^^ > -virNetClientSetCloseCallback(mon->client, virLXCMonitorEOFNotify, mon, > - virLXCMonitorCloseFreeCallback); > > cleanup: > VIR_FREE(sockpath); > isn't it needed because of this SetCloseCallback, or actually virLXCMonitorCloseFreeCallback()? The extra reference is held around so that virLXCMonitorEOFNotify can use it and it's then released in virLXCMonitorCloseFreeCallback(). If you move the SetCloseCallback() call and for instance virNetClientProgramNew() fails, then I think we are facing a double free: one free() is right under error label, the other is when the connection closes. Therefore I think we have to move the ref call too. No need to post v2, I am just checking with you. I can fix it before pushing. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 07/13] conf: Introduce parser, formatter for uid and fid
在 2018/9/25 下午4:02, Andrea Bolognani 写道: On Tue, 2018-09-25 at 13:15 +0800, Yi Min Zhao wrote: From the high level point of view, code outside of conf/ should, for the most part, not need to concern itself with zPCI at all: it would eg. ask for a PCI address to be allocated, and if the device in question can be a zPCI device then a zPCI extension address will be allocated for it as part of the same function call; the only part of qemu/ that should care about the zPCI address is the one generating the relevant command line arguments. Can you try and see whether this kind of API would work? I did a simple test. It worked. Do you prefer this way? Yes please, I'd very much like to see what that looks like and whether it addresses the problems caused by the ambiguity of the API we've used until now. So do you mean we use this kind of API in next version for review? Yes, please :) I'm not sure how big the change is. So I might take more time to prepare the new version. So that we can't catch up with the new release. Thanks for your kindness and comments! -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 29/47] vircgroup: extract virCgroupV1SetMemory
On Tue, Sep 18, 2018 at 05:45:50PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 20 +--- src/util/vircgroupbackend.h | 6 ++ src/util/vircgroupv1.c | 28 3 files changed, 35 insertions(+), 19 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 28/47] vircgroup: extract virCgroupV1(Set|Get)BlkioDeviceWriteBps
On Tue, Sep 18, 2018 at 05:45:49PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 34 ++--- src/util/vircgroupbackend.h | 12 + src/util/vircgroupv1.c | 50 + 3 files changed, 64 insertions(+), 32 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 27/47] vircgroup: extract virCgroupV1(Set|Get)BlkioDeviceReadBps
On Tue, Sep 18, 2018 at 05:45:48PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 34 ++--- src/util/vircgroupbackend.h | 12 + src/util/vircgroupv1.c | 50 + 3 files changed, 64 insertions(+), 32 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 26/47] vircgroup: extract virCgroupV1(Set|Get)BlkioDeviceWriteIops
On Tue, Sep 18, 2018 at 05:45:47PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 34 ++--- src/util/vircgroupbackend.h | 12 + src/util/vircgroupv1.c | 50 + 3 files changed, 64 insertions(+), 32 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 25/47] vircgroup: extract virCgroupV1(Set|Get)BlkioDeviceReadIops
On Tue, Sep 18, 2018 at 05:45:46PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 34 ++--- src/util/vircgroupbackend.h | 12 + src/util/vircgroupv1.c | 50 + 3 files changed, 64 insertions(+), 32 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 24/47] vircgroup: extract virCgroupV1(Set|Get)BlkioDeviceWeight
On Tue, Sep 18, 2018 at 05:45:45PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 36 +++--- src/util/vircgroupbackend.h | 12 + src/util/vircgrouppriv.h| 6 + src/util/vircgroupv1.c | 50 + 4 files changed, 71 insertions(+), 33 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/47] vircgroup: extract virCgroupV1GetBlkioIoDeviceServiced
On Tue, Sep 18, 2018 at 05:45:44PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 83 ++ src/util/vircgroupbackend.h | 9 src/util/vircgrouppriv.h| 2 + src/util/vircgroupv1.c | 89 + 4 files changed, 104 insertions(+), 79 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 4b9c03830c..206318d436 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1049,6 +1049,94 @@ virCgroupV1GetBlkioIoServiced(virCgroupPtr group, } +static int +virCgroupV1GetBlkioIoDeviceServiced(virCgroupPtr group, +const char *path, +long long *bytes_read, +long long *bytes_write, +long long *requests_read, +long long *requests_write) +{ +VIR_AUTOFREE(char *) str1 = NULL; +VIR_AUTOFREE(char *) str2 = NULL; +VIR_AUTOFREE(char *) str3 = NULL; Just like in the last patch. Instead of: +char *p1, *p2; use: -char *p1 = NULL; -char *p2 = NULL; +size_t i; + +const char *value_names[] = { +"Read ", +"Write " +}; Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 22/47] vircgroup: extract virCgroupV1GetBlkioIoServiced
On Tue, Sep 18, 2018 at 05:45:43PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 87 ++ src/util/vircgroupbackend.h | 8 src/util/vircgroupv1.c | 94 + 3 files changed, 105 insertions(+), 84 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 380a310589..4b9c03830c 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -956,6 +956,99 @@ virCgroupV1GetBlkioWeight(virCgroupPtr group, } +static int +virCgroupV1GetBlkioIoServiced(virCgroupPtr group, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ +long long stats_val; +VIR_AUTOFREE(char *) str1 = NULL; +VIR_AUTOFREE(char *) str2 = NULL; +char *p1, *p2; These were written as: -char *p1 = NULL; -char *p2 = NULL; in the original function. Perhaps incorrect conflict resolution with commit 94f1855f util: cgroup: use VIR_AUTOFREE instead of VIR_FREE for scalar types +size_t i; + +const char *value_names[] = { +"Read ", +"Write " +}; With that addressed: Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 07/13] conf: Introduce parser, formatter for uid and fid
On Tue, 2018-09-25 at 13:15 +0800, Yi Min Zhao wrote: > > > > From the high level point of view, code outside of conf/ should, > > > > for the most part, not need to concern itself with zPCI at all: it > > > > would eg. ask for a PCI address to be allocated, and if the device > > > > in question can be a zPCI device then a zPCI extension address will > > > > be allocated for it as part of the same function call; the only > > > > part of qemu/ that should care about the zPCI address is the one > > > > generating the relevant command line arguments. > > > > > > > > Can you try and see whether this kind of API would work? > > > > > > I did a simple test. It worked. Do you prefer this way? > > > > Yes please, I'd very much like to see what that looks like and > > whether it addresses the problems caused by the ambiguity of the > > API we've used until now. > > So do you mean we use this kind of API in next version for review? Yes, please :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 21/47] vircgroup: extract virCgroupV1(Set|Get)BlkioWeight
On Tue, Sep 18, 2018 at 05:45:42PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c| 14 ++ src/util/vircgroupbackend.h | 20 src/util/vircgroupv1.c | 29 + 3 files changed, 51 insertions(+), 12 deletions(-) @@ -161,4 +173,12 @@ virCgroupBackendRegister(virCgroupBackendPtr backend); virCgroupBackendPtr * virCgroupBackendGetAll(void); +# define VIR_CGROUP_BACKEND_CALL(group, func, ret, ...) \ +if (!group->backend->func) { \ +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", \ + _("operation '" #func "' not supported")); \ _("operation '%s' not supported"), #func) I don't think the function names are translatable. And having a separate translatable string for each unlikely error message is a waste of translators' time. +return ret; \ +} \ +return group->backend->func(group, ##__VA_ARGS__); + #endif /* __VIR_CGROUP_BACKEND_H__ */ Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plans for next release
On 09/24/2018 03:42 PM, Daniel Veillard wrote: > that's next week ! If we want to release around Oct 1st, I would > suggest to enter freeze this Wed, probably in the morning europe time, > then plan for an RC2 friday morning and if everything goes well, have > the final release next monday. > > I hope that works for everybody ! Well, I introduced metadata locking in this release and as usual there are some follow up patches waiting for review: https://www.redhat.com/archives/libvir-list/2018-September/msg01134.html Also, Bjoern reported some issues with the feature (/me wonders if it's s390 specific) and he said he'll debug this week. Anyway, those patches and any patch that he'll write can be viewed as a bug fix and therefore can be merged during freeze. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/4] security: Don't try to lock NULL paths
It may happen that in the list of paths/disk sources to relabel there is a disk source. If that is the case, the path is NULL. In that case, we shouldn't try to lock the path. It's likely a network disk anyway and therefore there is nothing to lock. Signed-off-by: Michal Privoznik --- src/security/security_dac.c | 3 ++- src/security/security_selinux.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 876cca0f2f..04168feb3d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -216,7 +216,8 @@ virSecurityDACTransactionRun(pid_t pid ATTRIBUTE_UNUSED, for (i = 0; i < list->nItems; i++) { const char *p = list->items[i]->path; -if (virFileIsDir(p)) +if (!p || +virFileIsDir(p)) continue; VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 3c847d8dcb..3adbeada14 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -227,7 +227,8 @@ virSecuritySELinuxTransactionRun(pid_t pid ATTRIBUTE_UNUSED, for (i = 0; i < list->nItems; i++) { const char *p = list->items[i]->path; -if (virFileIsDir(p)) +if (!p || +virFileIsDir(p)) continue; VIR_APPEND_ELEMENT_COPY_INPLACE(paths, npaths, p); -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list