Re: [libvirt] [PATCH v2 1/8] Added public API to enable post-copy migration
Hi everyone, On Thu, Nov 06, 2014 at 09:18:04AM +0200, Cristian Klein wrote: I > talked to the qemu post-copy guys (Andrea and Dave in CC). Starting > post-copy immediately is a bad performance choice: The VM will start > on the destination hypervisor before the read-only or kernel memory > is there. This means that those pages need to be pulled on-demand, > hence a lot of overhead and interruptions in the VM’s execution. > > Instead, it is better to first do one pass of pre-copy and only then > trigger post-copy. In fact, I did an experiment with a video > streaming VM and starting post-copy after the first pass of pre-copy > (instead of starting post-copy immediately) reduces downtime from > 3.5 seconds to under 1 second. > > Given all above, I propose the following post-copy API in libvirt: > > virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY) > virDomainMigrateStartPostCopy(...) // from a different thread > > This is for those who just need the post-copy mechanism and want to > implement a policy themselves. > > > virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY) > > This is for those who want to use post-copy without caring about any > low-level details, offering a good enough policy for most cases. > > What do you think? Would you accept patches that implement this API? I agree, even better would be to also pass a parameter to specify how many passes of precopy to run before engaging postcopy, adding at least the number of passes parameter shouldn't be a huge change and in doubt you can just define it to 1. The other things needed are: 1) adding an event that doesn't require libvirt to poll to know when the source node has been stopped (then if postcopy was engaged precopy may not have finished, but the problem remains the same as with pure precopy: we need to know efficiently when exactly the source node has been stopped without adding an average 25msec latency) 2) preparing a second socket for qemu so we can run the out of band requests of postcopy without incurring into artificial latencies created by the socket sendbuffer kept filled by the background transfer (the hack to decrease /proc/sys/net/ipv4/tcp_wmem helps tremendously, but it's unlikely to ever be as efficient as having two sockets, potentially both running on openssl etc..) Thanks, Andrea -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] CVE-2014-7823: dumpxml: security hole with migratable flag
On 11/05/2014 05:30 PM, Eric Blake wrote: > Commit 28f8dfd (v1.0.0) introduced a security hole: in at least > the qemu implementation of virDomainGetXMLDesc, the use of the > flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only > connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE > prior to calling qemuDomainFormatXML. However, the use of > VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write > clients only. This patch treats the migratable flag as requiring > the same permissions, rather than analyzing what might break if > migratable xml no longer includes secret information. > > Fortunately, the information leak is low-risk: all that is gated > by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password; > but VNC passwords are already weak (FIPS forbids their use, and > on a non-FIPS machine, anyone stupid enough to trust a max-8-byte > password sent in plaintext over the network deserves what they > get). SPICE offers better security than VNC, and all other > secrets are properly protected by use of virSecret associations > rather than direct output in domain XML. > > * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC): > Tighten rules on use of migratable flag. > * src/libvirt-domain.c (virDomainGetXMLDesc): Likewise. > > Signed-off-by: Eric Blake > --- > > The libvirt-security list agreed that this did not need an embargo > because it is low-risk; but I'm on the road this week, so while > this patch for master can go in now, I won't complete the backport > to all the affected stable branches (everything since v1.0.0) or > do the Libvirt Security Notice writeup until Monday. Pushed based on positive review on the libvirt-security list. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/8] Added public API to enable post-copy migration
On 01 Oct 2014, at 12:07 , Jiri Denemark wrote: > On Wed, Oct 01, 2014 at 10:45:33 +0200, Cristian KLEIN wrote: >> On 2014-09-30 17:16, Daniel P. Berrange wrote: >>> On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote: On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote: > Signed-off-by: Cristian Klein > --- > include/libvirt/libvirt.h.in | 1 + > src/libvirt.c| 7 +++ > 2 files changed, 8 insertions(+) > > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in > index 5217ab3..82f3aeb 100644 > --- a/include/libvirt/libvirt.h.in > +++ b/include/libvirt/libvirt.h.in > @@ -1225,6 +1225,7 @@ typedef enum { > VIR_MIGRATE_ABORT_ON_ERROR= (1 << 12), /* abort migration on I/O > errors happened during migration */ > VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ > VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ > +VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't > start) post-copy */ > } virDomainMigrateFlags; I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge. >>> >>> Your point about spawning another thread makes me wonder if we should >>> actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require >>> P2P migration of course). If this flag were set, virDomainMigrateXXX would >>> only block for long enough to start the migration and then return. >>> >>> Callers can use the job info API to monitor progress & success/failure. >>> >>> Then we wouldn't have to keep adding flags like you suggest - apps can >>> just easily call the appropriate API right away with no threads needed >> >> This would make a lot of sense. The user would call: >> >> """ >> virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC) >> virDomainMigrateStartPostCopy(...) >> """ >> >> Would this be seen as more cumbersome than having a dedicated >> VIR_MIGRATE_POSTCOPY_AUTOSTART? > > The ASYNC flag Daniel suggested makes sense, so I guess you can just > ignore my request for a special flag. Although, I don't think the ASYNC > stuff needs to be done within this series, let's just focus on the > post-copy stuff. Hi Jirka, I talked to the qemu post-copy guys (Andrea and Dave in CC). Starting post-copy immediately is a bad performance choice: The VM will start on the destination hypervisor before the read-only or kernel memory is there. This means that those pages need to be pulled on-demand, hence a lot of overhead and interruptions in the VM’s execution. Instead, it is better to first do one pass of pre-copy and only then trigger post-copy. In fact, I did an experiment with a video streaming VM and starting post-copy after the first pass of pre-copy (instead of starting post-copy immediately) reduces downtime from 3.5 seconds to under 1 second. Given all above, I propose the following post-copy API in libvirt: virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY) virDomainMigrateStartPostCopy(...) // from a different thread This is for those who just need the post-copy mechanism and want to implement a policy themselves. virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY) This is for those who want to use post-copy without caring about any low-level details, offering a good enough policy for most cases. What do you think? Would you accept patches that implement this API? Cristian -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable
On Wed, Nov 05, 2014 at 06:07:18PM +0100, Michal Privoznik wrote: And yet again one fix of 90286418. The problem is, if libvirt is built with NUMA disabled (--without-numad --without-numactl), the testsuite doesn't count on that and some tests fail. This is because the virNumaNodesetIsAvailable() function is taken from another section of virnuma.c which contains functions stubs for case NUMA is disabled at build time. And the function basically yields an error. However, we want our testsuite to be deterministic and hence we ought to mock the function. Signed-off-by: Michal Privoznik --- tests/qemuxml2argvmock.c | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 7218747..7b8bdd6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -39,3 +39,12 @@ virNumaGetMaxNode(void) return maxnodesNum; } + +bool +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) +{ +int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); +int maxHostNode = virNumaGetMaxNode(); + +return maxGuestNode <= maxHostNode; +} -- 2.0.4 NACK, we shouldn't need to mock this function. I went over the commit once more and I've realized it doesn't support non-contiguous nodesets even if it could. And since it will use only functions that have both versions (better one with NUMA support and a stub without it) it can be moved out of those #ifdefs. I have the patch almost ready :) I'll also remove the need for util to have connection to conf (it will make more sense, hopefully. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable
On 11/05/14 18:07, Michal Privoznik wrote: > And yet again one fix of 90286418. The problem is, if libvirt is > built with NUMA disabled (--without-numad --without-numactl), the > testsuite doesn't count on that and some tests fail. This is > because the virNumaNodesetIsAvailable() function is taken from > another section of virnuma.c which contains functions stubs for > case NUMA is disabled at build time. And the function basically > yields an error. However, we want our testsuite to be > deterministic and hence we ought to mock the function. > > Signed-off-by: Michal Privoznik > --- > tests/qemuxml2argvmock.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c > index 7218747..7b8bdd6 100644 > --- a/tests/qemuxml2argvmock.c > +++ b/tests/qemuxml2argvmock.c > @@ -39,3 +39,12 @@ virNumaGetMaxNode(void) > > return maxnodesNum; > } > + > +bool > +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) > +{ > +int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); > +int maxHostNode = virNumaGetMaxNode(); > + > +return maxGuestNode <= maxHostNode; > +} > The reason the function needs to be in the conditionally compiled section is that the function uses the NUMA_NUM_NODES macro. The rest of the used symbols has a stub and/or is already mocked. So we need to mock this too unfortunately. ACK Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] private.syms: Export virDomainNumatuneSpecifiedMaxNode
On 11/05/14 18:07, Michal Privoznik wrote: > As of 90286418 the function is introduced. However, it's missing > an entry in the libvirt_private.syms so it can't be mocked. > > Signed-off-by: Michal Privoznik > --- > src/libvirt_private.syms | 1 + > 1 file changed, 1 insertion(+) > ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virnuma: Add some more comments
On 11/05/14 18:07, Michal Privoznik wrote: > Especially add comments to mark ifdef, else and endif sections. 'Especially' sounds a bit odd here since the comments you are adding are only else/ifdef macros. > > Signed-off-by: Michal Privoznik > --- > src/util/virnuma.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > ACK Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes
On Wednesday 05 November 2014 08:40 PM, Michal Privoznik wrote: > On 05.11.2014 11:56, Prerna Saxena wrote: >> This patch set addresses a bunch of memory & NUMA fixes. >> >> >> Series Description: >> === >> Patch 1/3 : Use consistent data type to represent memory elements in various >> XML attributes. This ensures all memory elements are always represented as >> 'unsigned long long'. >> Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of >> a NUMA cell. This enables users to easily describe how much memory needs to >> be allocated to each NUMA cell for a guest >> domain. >> Patch 3/3 : This augments test cases to add the 'unit' tag. >> >> Regards, >> > The 2/3 and 3/3 should be merged together so that 'make check' won't fail > after 2/3. Otherwise looking good. I've merged 1/3 already. > Thanks Michal ! I'll merge the remaining two and send out a cumulative patch :) Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] qemu: revert patch - bandwidth tuning in session mode
On 05.11.2014 18:41, Erik Skultety wrote: Since there was a valid note to patch 43b67f2e about the best spot to check for bandwidth set call while having libvirt daemon run in session mode, this patch reverts previous changes dealing with bandwith (also reverts adding variable @cfg in qemuDomainGetNumaParameters which does not have any use at the moment, but getting and unreferencing driver's config) in qemu_driver.c and qemu_command.c. There will be another patch in the series which introduces the fix itself. --- src/qemu/qemu_command.c | 11 --- src/qemu/qemu_driver.c | 9 - 2 files changed, 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..956bb14 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7841,17 +7841,6 @@ qemuBuildCommandLine(virConnectPtr conn, _("CPU tuning is not available in session mode")); goto error; } - -virDomainNetDefPtr *nets = def->nets; -virNetDevBandwidthPtr bandwidth = NULL; -size_t nnets = def->nnets; -for (i = 0; i < nnets; i++) { -if ((bandwidth = virDomainNetGetActualBandwidth(nets[i])) != NULL) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -_("Network bandwidth tuning is not available in session mode")); -goto error; -} -} } for (i = 0; i < def->ngraphics; ++i) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..edd82c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9383,7 +9383,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; -virQEMUDriverConfigPtr cfg = NULL; char *nodeset = NULL; int ret = -1; virCapsPtr caps = NULL; @@ -9402,7 +9401,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, return -1; priv = vm->privateData; -cfg = virQEMUDriverGetConfig(driver); if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -9476,7 +9474,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, if (vm) virObjectUnlock(vm); virObjectUnref(caps); -virObjectUnref(cfg); return ret; } @@ -10447,12 +10444,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (virDomainSetInterfaceParametersEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; -if (!cfg->privileged) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Network bandwidth tuning is not available in session mode")); -goto cleanup; -} - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; ACK, however, I'm not pushing this one until 2/2 is fixed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] Iface: disallow network tuning in session mode globally
On 05.11.2014 18:41, Erik Skultety wrote: Patch 43b67f2e disallowed network tuning only with qemu driver, however this patch moved the check for root privileges into virNetDevBandwidthSet function, so the call should now fail in all possible cases. A mock function was created so that the test suite doesn't fail because of unsufficient privileges. --- src/util/virnetdevbandwidth.c | 8 tests/Makefile.am | 11 ++- tests/virnetdevbandwidthtest.c | 14 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 5fa231a..8360fd4 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virerror.h" #include "virstring.h" +#include "unistd.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -74,6 +75,13 @@ virNetDevBandwidthSet(const char *ifname, goto cleanup; } +if (geteuid() != 0) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Network bandwidth tuning is not available" + " in session mode")); +return -1; +} + virNetDevBandwidthClear(ifname); if (bandwidth->in && bandwidth->in->average) { diff --git a/tests/Makefile.am b/tests/Makefile.am index 7b22f90..7fa4575 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -392,6 +392,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevbandwidthmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -409,7 +410,9 @@ test_libraries += \ endif WITH_DBUS if WITH_LINUX -test_libraries += virusbmock.la +test_libraries += \ + virusbmock.la + $(NULL) endif WITH_LINUX This chunk seem unrelated. if WITH_TESTS @@ -829,6 +832,12 @@ virnetdevbandwidthtest_SOURCES = \ virnetdevbandwidthtest.c testutils.h testutils.c virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS) +virnetdevbandwidthmock_la_SOURCES = \ + virnetdevbandwidthmock.c +virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS) +virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \ +-rpath /evil/libtool/hack/to/force/shared/lib/creation + Did you forget to 'git add virnetdevbandwidthmock.c'? It's not in the patch anywhere. virkmodtest_SOURCES = \ virkmodtest.c testutils.h testutils.c virkmodtest_LDADD = $(LDADDS) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 384991e..df69bac 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -21,6 +21,9 @@ #include #include "testutils.h" + +#ifdef WITH_LINUX + #define __VIR_COMMAND_PRIV_H_ALLOW__ #include "vircommandpriv.h" #include "virnetdevbandwidth.h" @@ -167,4 +170,13 @@ mymain(void) return ret; } -VIRT_TEST_MAIN(mymain); +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevbandwidthmock.so") + +#else + +int main(void) +{ +return EXIT_AM_SKIP; +} + +#endif /* WITH_LINUX */ Okay, we can make this test run on Linux only. Well, I don't think they have tc in *BSD anyway, do they? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] UpdateDevice: Allow startupPolicy update
On 05.11.2014 16:29, Peter Krempa wrote: Add qemu: prefix to subject and mention that it's only with the _CONFIG flag. Fixed. On 11/05/14 14:11, Michal Privoznik wrote: Users might want to update startupPolicy via the virDomainUpdateDeviceFlags API too. This patch implements the feature on config layer. Signed-off-by: Michal Privoznik --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..6fc15c0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, if (disk->src->format) orig->src->format = disk->src->format; disk->src->path = NULL; +orig->startupPolicy = disk->startupPolicy; Looks on par with what we are doing now. I wanted to comment that if you don't specify a startup policy you might want to keep the existing one, but the code does not use this approach, thus ... Well, in order to do that we would need to allow the following: startupPolicy='default' to allow users to remove already set startupPolicy. But that's not supported currently, so I went the other way. break; case VIR_DOMAIN_DEVICE_NET: ACK with the subject tweaked Pushed now. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] Iface: disallow network tuning in session mode globally
Patch 43b67f2e disallowed network tuning only with qemu driver, however this patch moved the check for root privileges into virNetDevBandwidthSet function, so the call should now fail in all possible cases. A mock function was created so that the test suite doesn't fail because of unsufficient privileges. --- src/util/virnetdevbandwidth.c | 8 tests/Makefile.am | 11 ++- tests/virnetdevbandwidthtest.c | 14 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 5fa231a..8360fd4 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virerror.h" #include "virstring.h" +#include "unistd.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -74,6 +75,13 @@ virNetDevBandwidthSet(const char *ifname, goto cleanup; } +if (geteuid() != 0) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Network bandwidth tuning is not available" + " in session mode")); +return -1; +} + virNetDevBandwidthClear(ifname); if (bandwidth->in && bandwidth->in->average) { diff --git a/tests/Makefile.am b/tests/Makefile.am index 7b22f90..7fa4575 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -392,6 +392,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevbandwidthmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -409,7 +410,9 @@ test_libraries += \ endif WITH_DBUS if WITH_LINUX -test_libraries += virusbmock.la +test_libraries += \ + virusbmock.la + $(NULL) endif WITH_LINUX if WITH_TESTS @@ -829,6 +832,12 @@ virnetdevbandwidthtest_SOURCES = \ virnetdevbandwidthtest.c testutils.h testutils.c virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS) +virnetdevbandwidthmock_la_SOURCES = \ + virnetdevbandwidthmock.c +virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS) +virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \ +-rpath /evil/libtool/hack/to/force/shared/lib/creation + virkmodtest_SOURCES = \ virkmodtest.c testutils.h testutils.c virkmodtest_LDADD = $(LDADDS) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 384991e..df69bac 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -21,6 +21,9 @@ #include #include "testutils.h" + +#ifdef WITH_LINUX + #define __VIR_COMMAND_PRIV_H_ALLOW__ #include "vircommandpriv.h" #include "virnetdevbandwidth.h" @@ -167,4 +170,13 @@ mymain(void) return ret; } -VIRT_TEST_MAIN(mymain); +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevbandwidthmock.so") + +#else + +int main(void) +{ +return EXIT_AM_SKIP; +} + +#endif /* WITH_LINUX */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] network: bandwidth tuning in session mode revert patch
Erik Skultety (2): qemu: revert patch - bandwidth tuning in session mode Iface: disallow network tuning in session mode globally src/qemu/qemu_command.c| 11 --- src/qemu/qemu_driver.c | 9 - src/util/virnetdevbandwidth.c | 8 tests/Makefile.am | 11 ++- tests/virnetdevbandwidthtest.c | 14 +- 5 files changed, 31 insertions(+), 22 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] qemu: revert patch - bandwidth tuning in session mode
Since there was a valid note to patch 43b67f2e about the best spot to check for bandwidth set call while having libvirt daemon run in session mode, this patch reverts previous changes dealing with bandwith (also reverts adding variable @cfg in qemuDomainGetNumaParameters which does not have any use at the moment, but getting and unreferencing driver's config) in qemu_driver.c and qemu_command.c. There will be another patch in the series which introduces the fix itself. --- src/qemu/qemu_command.c | 11 --- src/qemu/qemu_driver.c | 9 - 2 files changed, 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..956bb14 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7841,17 +7841,6 @@ qemuBuildCommandLine(virConnectPtr conn, _("CPU tuning is not available in session mode")); goto error; } - -virDomainNetDefPtr *nets = def->nets; -virNetDevBandwidthPtr bandwidth = NULL; -size_t nnets = def->nnets; -for (i = 0; i < nnets; i++) { -if ((bandwidth = virDomainNetGetActualBandwidth(nets[i])) != NULL) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -_("Network bandwidth tuning is not available in session mode")); -goto error; -} -} } for (i = 0; i < def->ngraphics; ++i) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..edd82c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9383,7 +9383,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; -virQEMUDriverConfigPtr cfg = NULL; char *nodeset = NULL; int ret = -1; virCapsPtr caps = NULL; @@ -9402,7 +9401,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, return -1; priv = vm->privateData; -cfg = virQEMUDriverGetConfig(driver); if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -9476,7 +9474,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, if (vm) virObjectUnlock(vm); virObjectUnref(caps); -virObjectUnref(cfg); return ret; } @@ -10447,12 +10444,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (virDomainSetInterfaceParametersEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; -if (!cfg->privileged) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Network bandwidth tuning is not available in session mode")); -goto cleanup; -} - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Fix build without NUMA
Even though these patches would qualify as build breaker fixes, I don't think they are that critical to be pushed without somebody else's taking a look on them. Michal Privoznik (3): virnuma: Add some more comments private.syms: Export virDomainNumatuneSpecifiedMaxNode qemuxml2argvmock: Mock virNumaNodesetIsAvailable src/libvirt_private.syms | 1 + src/util/virnuma.c | 14 +++--- tests/qemuxml2argvmock.c | 9 + 3 files changed, 17 insertions(+), 7 deletions(-) -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] private.syms: Export virDomainNumatuneSpecifiedMaxNode
As of 90286418 the function is introduced. However, it's missing an entry in the libvirt_private.syms so it can't be mocked. Signed-off-by: Michal Privoznik --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 73b8167..8895ba1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -609,6 +609,7 @@ virDomainNumatuneParseXML; virDomainNumatunePlacementTypeFromString; virDomainNumatunePlacementTypeToString; virDomainNumatuneSet; +virDomainNumatuneSpecifiedMaxNode; # conf/nwfilter_conf.h -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virnuma: Add some more comments
Especially add comments to mark ifdef, else and endif sections. Signed-off-by: Michal Privoznik --- src/util/virnuma.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 89435de..ee1c4af 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -74,7 +74,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus, virCommandFree(cmd); return output; } -#else +#else /* !HAVE_NUMAD */ char * virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED, unsigned long long balloon ATTRIBUTE_UNUSED) @@ -83,7 +83,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED, _("numad is not available on this host")); return NULL; } -#endif +#endif /* !HAVE_NUMAD */ #if WITH_NUMACTL int @@ -339,7 +339,8 @@ virNumaGetNodeCPUs(int node, # undef MASK_CPU_ISSET # undef n_bits -#else +#else /* !WITH_NUMACTL */ + int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune, virBitmapPtr nodemask ATTRIBUTE_UNUSED) @@ -404,8 +405,7 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED, _("NUMA isn't available on this host")); return -1; } -#endif - +#endif /* !WITH_NUMACTL */ /** * virNumaGetMaxCPUs: @@ -494,8 +494,8 @@ virNumaGetDistances(int node, return ret; } +#else /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */ -#else bool virNumaNodeIsAvailable(int node) { @@ -519,7 +519,7 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, VIR_DEBUG("NUMA distance information isn't available on this host"); return 0; } -#endif +#endif /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */ /* currently all the huge page stuff below is linux only */ -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable
And yet again one fix of 90286418. The problem is, if libvirt is built with NUMA disabled (--without-numad --without-numactl), the testsuite doesn't count on that and some tests fail. This is because the virNumaNodesetIsAvailable() function is taken from another section of virnuma.c which contains functions stubs for case NUMA is disabled at build time. And the function basically yields an error. However, we want our testsuite to be deterministic and hence we ought to mock the function. Signed-off-by: Michal Privoznik --- tests/qemuxml2argvmock.c | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 7218747..7b8bdd6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -39,3 +39,12 @@ virNumaGetMaxNode(void) return maxnodesNum; } + +bool +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) +{ +int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); +int maxHostNode = virNumaGetMaxNode(); + +return maxGuestNode <= maxHostNode; +} -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] CVE-2014-7823: dumpxml: security hole with migratable flag
Commit 28f8dfd (v1.0.0) introduced a security hole: in at least the qemu implementation of virDomainGetXMLDesc, the use of the flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE prior to calling qemuDomainFormatXML. However, the use of VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write clients only. This patch treats the migratable flag as requiring the same permissions, rather than analyzing what might break if migratable xml no longer includes secret information. Fortunately, the information leak is low-risk: all that is gated by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password; but VNC passwords are already weak (FIPS forbids their use, and on a non-FIPS machine, anyone stupid enough to trust a max-8-byte password sent in plaintext over the network deserves what they get). SPICE offers better security than VNC, and all other secrets are properly protected by use of virSecret associations rather than direct output in domain XML. * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC): Tighten rules on use of migratable flag. * src/libvirt-domain.c (virDomainGetXMLDesc): Likewise. Signed-off-by: Eric Blake --- The libvirt-security list agreed that this did not need an embargo because it is low-risk; but I'm on the road this week, so while this patch for master can go in now, I won't complete the backport to all the affected stable branches (everything since v1.0.0) or do the Libvirt Security Notice writeup until Monday. src/libvirt-domain.c | 3 ++- src/remote/remote_protocol.x | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..2b0defc 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2607,7 +2607,8 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) virCheckDomainReturn(domain, NULL); conn = domain->conn; -if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) { +if ((conn->flags & VIR_CONNECT_RO) && +(flags & (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) { virReportError(VIR_ERR_OPERATION_DENIED, "%s", _("virDomainGetXMLDesc with secure flag")); goto error; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db12cda..ebf4530 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3255,6 +3255,7 @@ enum remote_procedure { * @generate: both * @acl: domain:read * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE + * @acl: domain:read_secure:VIR_DOMAIN_XML_MIGRATABLE */ REMOTE_PROC_DOMAIN_GET_XML_DESC = 14, -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] UpdateDevice: Allow startupPolicy update
Add qemu: prefix to subject and mention that it's only with the _CONFIG flag. On 11/05/14 14:11, Michal Privoznik wrote: > Users might want to update startupPolicy via the > virDomainUpdateDeviceFlags API too. This patch > implements the feature on config layer. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_driver.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 6acaea8..6fc15c0 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, > if (disk->src->format) > orig->src->format = disk->src->format; > disk->src->path = NULL; > +orig->startupPolicy = disk->startupPolicy; Looks on par with what we are doing now. I wanted to comment that if you don't specify a startup policy you might want to keep the existing one, but the code does not use this approach, thus ... > break; > > case VIR_DOMAIN_DEVICE_NET: > ACK with the subject tweaked signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Memory: Use consistent type for all memory elements.
On 05.11.2014 11:58, Prerna Saxena wrote: From 4b3e336ea045759758b04440d75802e990506e2b Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Fri, 31 Oct 2014 16:07:21 +0530 Domain memory elements such as max_balloon and cur_balloon are implemented as 'unsigned long long', whereas the 'memory' element in NUMA cells is implemented as 'unsigned int'. Use the same data type (unsigned long long) for 'memory' element in NUMA cells. Signed-off-by: Prerna Saxena --- src/conf/cpu_conf.c | 4 ++-- src/conf/cpu_conf.h | 2 +- src/qemu/qemu_command.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 9b7fbb0..5475c07 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -496,7 +496,7 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); @@ -702,7 +702,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "cells[i].cpustr); -virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem); +virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d45210b..5bcf101 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -105,7 +105,7 @@ typedef virCellDef *virCellDefPtr; struct _virCellDef { virBitmapPtr cpumask; /* CPUs that are part of this node */ char *cpustr; /* CPUs stored in string form for dumpxml */ -unsigned int mem; /* Node memory in kB */ +unsigned long long mem; /* Node memory in kB */ virMemAccess memAccess; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..13b54dd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6693,7 +6693,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } for (i = 0; i < def->cpu->ncells; i++) { -int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); +unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; virMemAccess memAccess = def->cpu->cells[i].memAccess; @@ -6799,7 +6799,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAddLit(&buf, "memory-backend-ram"); } -virBufferAsprintf(&buf, ",size=%dM,id=ram-node%zu", cellmem, i); +virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i); if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset, &nodemask, i) < 0) @@ -6849,7 +6849,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); } else { -virBufferAsprintf(&buf, ",mem=%d", cellmem); +virBufferAsprintf(&buf, ",mem=%llu", cellmem); } virCommandAddArgBuffer(cmd, &buf); ACKed & pushed as this doesn't depend on the rest of the patches. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes
On 05.11.2014 11:56, Prerna Saxena wrote: This patch set addresses a bunch of memory & NUMA fixes. Series Description: === Patch 1/3 : Use consistent data type to represent memory elements in various XML attributes. This ensures all memory elements are always represented as 'unsigned long long'. Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of a NUMA cell. This enables users to easily describe how much memory needs to be allocated to each NUMA cell for a guest domain. Patch 3/3 : This augments test cases to add the 'unit' tag. Regards, The 2/3 and 3/3 should be merged together so that 'make check' won't fail after 2/3. Otherwise looking good. I've merged 1/3 already. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Numa : Allow specification of 'units' for numa nodes.
On 05.11.2014 12:06, Prerna Saxena wrote: From 7fe8487c5e8d24086637d2157bad25322b3654f7 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 07:53:59 +0530 CPU numa topology implicitly allows memory specification in 'KiB'. Enabling this to accept the 'unit' in which memory needs to be specified. This now allows users to specify memory in units of choice, and lists the same in 'KiB' -- just like other 'memory' elements in XML. I wanted to use virDomainParseScaledValue(), but that function implicitly assumes an XML layout where 'memory' is an _element_ of type 'scaledInteger', with 'unit' as its attribute. A NUMA cell has XML specification where 'memory' is an _attribute_ of element 'cell'. Since changing XML layout is not an option, I have borrowed code from the same. Yeah, I did the same in virDomainHugepagesParseXML. Maybe you can rename it to something more generic and reuse it here? Looking forward to suggestions on how this can best be done. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 5 + src/conf/cpu_conf.c | 36 ++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..44cabad 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4144,6 +4144,11 @@ + + + + + Any XML change requires docs update. I can't ACK this with lacking documentation, sorry. shared diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5475c07..65b9815 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -189,6 +189,7 @@ virCPUDefParseXML(xmlNodePtr node, char *cpuMode; char *fallback = NULL; char *vendor_id = NULL; +unsigned long long max; if (!xmlStrEqual(node->name, BAD_CAST "cpu")) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -439,10 +440,20 @@ virCPUDefParseXML(xmlNodePtr node, def->ncells = n; +/* 32 vs 64 bit will differ in value of upper memory bound. + * On 32-bit machines, our bound is 0x * KiB. On 64-bit + * machines, our bound is off_t (2^63). + */ +if (sizeof(unsigned long) < sizeof(long long)) +max = 1024ull * ULONG_MAX; +else +max = LLONG_MAX; + for (i = 0; i < n; i++) { -char *cpus, *memory, *memAccessStr; +char *cpus, *memory, *memAccessStr, *unit; int ret, ncpus = 0; unsigned int cur_cell; +unsigned long long bytes; char *tmp = NULL; tmp = virXMLPropString(nodes[i], "id"); @@ -496,14 +507,34 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &bytes); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); VIR_FREE(memory); goto error; } + +unit = virXMLPropString(nodes[i], "unit"); +if (!unit) { +if (VIR_STRDUP(unit, "KiB") < 0) +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error processing 'memory' in NUMA cell")); +} + +if (virScaleInteger(&bytes, unit, 1024, max) < 0) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("Error scaling 'memory' in NUMA cell")); +VIR_FREE(memory); +VIR_FREE(unit); +goto error; +} + +def->cells[cur_cell].mem = VIR_DIV_UP(bytes, 1024); + +bytes = 0; VIR_FREE(memory); +VIR_FREE(unit); memAccessStr = virXMLPropString(nodes[i], "memAccess"); if (memAccessStr) { @@ -703,6 +734,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAsprintf(buf, " id='%zu'", i); virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr); virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); +virBufferAddLit(buf, " unit='KiB'"); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Local qemu migration
On Wed, Nov 5, 2014 at 2:54 PM, Eric Blake wrote: > The monitor socket location is different for qemu:///system than for > qemu:///session (or when going between two different users' > qemu:///session). But things like the VNC port do start to be an issue. > > Not if the port is allocated, or the unix path is namespaced. But I understand better where the problems may come from. Still, I would imagine that libvirt would simply fail to migrate and no bad things would happen in those cases. > > > Migration fundamentally requires that the two QEMU processes involved > have > > completely separate filesystem namespaces, which effectively means > separate > > hosts (or at least separate containers which is effectively the same > thing) > > > >> Except the obvious testing case, there are other use cases that could be > >> interesting: moving a running VM from system to session libvirt, or the > >> other way around, or to a different user. > > > > Migrating from session to system libvirt or vica-verca isn't something > > that is reasonable to support either because of the different privilege > > levels. The session QEMU will require the disk images to be owned by > > one user account, the system QEMU will require them to be owned by a > > different user account. We can't chown the images to satisfy both the > > system and session QEMU at the same time. > > This, coupled with the fact that we don't allow connection to a remote > qemu:///session instance (right now, the RPC code assumes that it will > only connect to qemu:///system), means that it is not possible to do > live migration in or out of a session instance, regardless of whether it > is on the same machine, and regardless of whether it is between the > session libvirtd of two different users. > Well, I managed to migrate from system to session with a disk less VM, it was useful for testing. All you have to do is to provide the unix socket path, ex: qemu+unix:///session?socket=/run/user/1000/libvirt/libvirt-sock Although it would be nice to make this easier to lookup eventually > > It might be possible to migrate to file (such as virsh save), then > update permissions on the save file and other resources, then reload > from that file; but it is not a live migration. > Yes, that's what Boxes is doing afaik. I should have stated clearly I was talking about live migration cheers -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: fix deadlock caused updating network device and nwfilter
Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine} and starting of guest, but this same deadlock is also for updating/attaching network device to domain. The deadlock was introduced by removing global QEMU driver lock because nwfilter was counting on this lock and ensure that all driver locks are locked inside of nwfilter{Define,Undefine}. This patch extends usage of virNWFilterReadLockFilterUpdates to prevent the deadlock for all possible paths in QEMU driver. LXC and UML drivers still have global lock. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143780 Signed-off-by: Pavel Hrdina --- This is temporary fix for the deadlock issue as I'm planning to create global libvirt jobs (similar to QEMU domain jobs) and use it for other drivers and for example for nwfilters too. src/qemu/qemu_driver.c| 12 src/qemu/qemu_migration.c | 3 +++ src/qemu/qemu_process.c | 4 3 files changed, 19 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..9e6f505 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5937,6 +5937,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, def = tmp; } +virNWFilterReadLockFilterUpdates(); + if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -5978,6 +5980,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, virFileWrapperFdFree(wrapperFd); if (vm) virObjectUnlock(vm); +virNWFilterUnlockFilterUpdates(); return ret; } @@ -7502,6 +7505,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG); +virNWFilterReadLockFilterUpdates(); + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; @@ -7614,6 +7619,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return ret; } @@ -7644,6 +7650,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); +virNWFilterReadLockFilterUpdates(); + cfg = virQEMUDriverGetConfig(driver); affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG); @@ -7760,6 +7768,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return ret; } @@ -14510,6 +14519,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * and use of FORCE can cause multiple transitions. */ +virNWFilterReadLockFilterUpdates(); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) return -1; @@ -14831,6 +14842,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 94a4cf6..18242ae 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2666,6 +2666,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto cleanup; } +virNWFilterReadLockFilterUpdates(); + if (!(vm = virDomainObjListAdd(driver->domains, *def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -2825,6 +2827,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); qemuMigrationCookieFree(mig); virObjectUnref(caps); +virNWFilterUnlockFilterUpdates(); return ret; stop: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 26d4948..409a672 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3422,6 +3422,8 @@ qemuProcessReconnect(void *opaque) VIR_FREE(data); +virNWFilterReadLockFilterUpdates(); + virObjectLock(obj); cfg = virQEMUDriverGetConfig(driver); @@ -3573,6 +3575,7 @@ qemuProcessReconnect(void *opaque) virObjectUnref(conn); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); return; @@ -3608,6 +3611,7 @@ qemuProcessReconnect(void *opaque) } virObjectUnref(conn); virObjectUnref(cfg); +virNWFilterUnlockFilterUpdates(); } static int -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Local qemu migration
On 11/05/2014 01:28 PM, Daniel P. Berrange wrote: > On Wed, Nov 05, 2014 at 01:16:08PM +0100, Marc-André Lureau wrote: >> Hi >> >> On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange >> wrote: >> >>> Nope, there's stuff that's on the filesystem that is tied to the QEMU >>> process and that would clash when migrating as you have two copies of >>> the same VM running at the sme time. >>> >> >> Ok, there is no guarantee that the VMs won't write concurrently to the disk >> during migration? >> >> Also, there are cases where VM disks/images are readonly, or image-less VMs. > > No, it isn't about the disk images. The actual guest CPUs are only executing > in one QEMU at a time, so there's not a problem wrt disk image usage. The > issue is with various other files we use. For example the QEMU monitor is a > UNIX domain socket at a specific path - you can't have both QEMUs have the > same UNIX domain socket. That's not under user control so we could change > the monitor socket path, but the problem extends to stuff that is directly > from the guest XML. ie VNC can be told to listen on a UNIX domain socket > path. virtio-serial devices are typically bound to UNIX domain sockets. > Serial ports, parallel ports, certain network device backends, and so on. The monitor socket location is different for qemu:///system than for qemu:///session (or when going between two different users' qemu:///session). But things like the VNC port do start to be an issue. > > Migration fundamentally requires that the two QEMU processes involved have > completely separate filesystem namespaces, which effectively means separate > hosts (or at least separate containers which is effectively the same thing) > >> Except the obvious testing case, there are other use cases that could be >> interesting: moving a running VM from system to session libvirt, or the >> other way around, or to a different user. > > Migrating from session to system libvirt or vica-verca isn't something > that is reasonable to support either because of the different privilege > levels. The session QEMU will require the disk images to be owned by > one user account, the system QEMU will require them to be owned by a > different user account. We can't chown the images to satisfy both the > system and session QEMU at the same time. This, coupled with the fact that we don't allow connection to a remote qemu:///session instance (right now, the RPC code assumes that it will only connect to qemu:///system), means that it is not possible to do live migration in or out of a session instance, regardless of whether it is on the same machine, and regardless of whether it is between the session libvirtd of two different users. It might be possible to migrate to file (such as virsh save), then update permissions on the save file and other resources, then reload from that file; but it is not a live migration. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: Fix memory leak in remoteConnectGetAllDomainStats
On 05.11.2014 12:45, Peter Krempa wrote: The remote call actually doesn't free the arguments array so we leak memory in case a domain list is specified. As the remote domain list array consists only of stolen pointers from the actual domain objects it's sufficient just to free the array. Valgrind message: ==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726 ==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618) ==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191) ==1081452==by 0x505D21E: remoteConnectGetAllDomainStats (remote_driver.c:7785) ==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080) ==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147) ==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935) ==1081452==by 0x133FEB: main (virsh.c:3719) --- src/remote/remote_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..d111e10 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, VIR_FREE(elem); } virDomainStatsRecordListFree(tmpret); +VIR_FREE(args.doms.doms_val); xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret, (char *) &ret); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] UpdateDevice: Allow startupPolicy update
Users might want to update startupPolicy via the virDomainUpdateDeviceFlags API too. This patch implements the feature on config layer. Signed-off-by: Michal Privoznik --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6acaea8..6fc15c0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, if (disk->src->format) orig->src->format = disk->src->format; disk->src->path = NULL; +orig->startupPolicy = disk->startupPolicy; break; case VIR_DOMAIN_DEVICE_NET: -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemu: Split qemuDomainSetTime into two functions
This is pure code movement to dig out the function internals into a separate functions as the code is to be reused later. Signed-off-by: Michal Privoznik --- src/qemu/qemu_driver.c | 86 +++--- 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b866ae8..903ca5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1868,6 +1868,59 @@ static int qemuDomainSuspend(virDomainPtr dom) } +/** + * qemuDomainSetTimeImpl: + * + * Domain must have a job set as the monitor is entered here. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuDomainSetTimeImpl(virQEMUDriverPtr driver, + virDomainObjPtr vm, + long long seconds, + unsigned int nseconds, + bool rtcSync) +{ +int rv, ret = -1; +qemuDomainObjPrivatePtr priv = vm->privateData; + +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot set time: qemu doesn't support " + "rtc-reset-reinjection command")); +goto cleanup; +} + +if (!qemuDomainAgentAvailable(priv, true)) +goto cleanup; + +qemuDomainObjEnterAgent(vm); +rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync); +qemuDomainObjExitAgent(vm); + +if (rv < 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); +goto cleanup; +} + +qemuDomainObjEnterMonitor(driver, vm); +rv = qemuMonitorRTCResetReinjection(priv->mon); +qemuDomainObjExitMonitor(driver, vm); + +if (rv < 0) +goto cleanup; + +ret = 0; + cleanup: +return ret; +} + + static int qemuDomainResumeFlags(virDomainPtr dom, unsigned int flags) @@ -17711,11 +17764,9 @@ qemuDomainSetTime(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; -qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; bool rtcSync = flags & VIR_DOMAIN_TIME_SYNC; int ret = -1; -int rv; virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret); @@ -17725,8 +17776,6 @@ qemuDomainSetTime(virDomainPtr dom, if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0) goto cleanup; -priv = vm->privateData; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -17736,34 +17785,7 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob; } -if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot set time: qemu doesn't support " - "rtc-reset-reinjection command")); -goto endjob; -} - -if (!qemuDomainAgentAvailable(priv, true)) -goto endjob; - -qemuDomainObjEnterAgent(vm); -rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync); -qemuDomainObjExitAgent(vm); - -if (rv < 0) -goto endjob; - -if (!virDomainObjIsActive(vm)) { -virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); -goto endjob; -} - -qemuDomainObjEnterMonitor(driver, vm); -rv = qemuMonitorRTCResetReinjection(priv->mon); -qemuDomainObjExitMonitor(driver, vm); - -if (rv < 0) +if (qemuDomainSetTimeImpl(driver, vm, seconds, nseconds, rtcSync) < 0) goto endjob; ret = 0; -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] Introduce virDomainResumeFlags API
The old DomainResume API lacks flags argument. This is unfortunate, because there may exist some use cases where an additional work could be done on domain resume. However, without flags it's not possible. Signed-off-by: Michal Privoznik --- include/libvirt/libvirt-domain.h | 2 ++ src/driver-hypervisor.h | 5 + src/libvirt-domain.c | 44 src/libvirt_public.syms | 5 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++- src/remote_protocol-structs | 5 + 7 files changed, 74 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b28d37d..1795dd3 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -869,6 +869,8 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend(virDomainPtr domain); int virDomainResume (virDomainPtr domain); +int virDomainResumeFlags(virDomainPtr domain, + unsigned int flags); int virDomainPMSuspendForDuration (virDomainPtr domain, unsigned int target, unsigned long long duration, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ad66629..e781475 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -116,6 +116,10 @@ typedef int (*virDrvDomainResume)(virDomainPtr domain); typedef int +(*virDrvDomainResumeFlags)(virDomainPtr domain, + unsigned int flags); + +typedef int (*virDrvDomainPMSuspendForDuration)(virDomainPtr, unsigned int target, unsigned long long duration, @@ -1205,6 +1209,7 @@ struct _virHypervisorDriver { virDrvDomainLookupByName domainLookupByName; virDrvDomainSuspend domainSuspend; virDrvDomainResume domainResume; +virDrvDomainResumeFlags domainResumeFlags; virDrvDomainPMSuspendForDuration domainPMSuspendForDuration; virDrvDomainPMWakeup domainPMWakeup; virDrvDomainShutdown domainShutdown; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6dcb9ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -693,6 +693,50 @@ virDomainResume(virDomainPtr domain) /** + * virDomainResumeFlags: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Resume a suspended domain, the process is restarted from the state where + * it was frozen by calling virDomainSuspend(). + * This function may require privileged access + * Moreover, resume may not be supported if domain is in some + * special state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainResumeFlags(virDomainPtr domain, + unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(domain); + +virResetLastError(); + +virCheckDomainReturn(domain, -1); +conn = domain->conn; + +virCheckReadOnlyGoto(conn->flags, error); + +if (conn->driver->domainResumeFlags) { +int ret; +ret = conn->driver->domainResumeFlags(domain, flags); +if (ret < 0) +goto error; +return ret; +} + +virReportUnsupportedError(); + + error: +virDispatchError(domain->conn); +return -1; +} + + +/** * virDomainPMSuspendForDuration: * @dom: a domain object * @target: a value from virNodeSuspendTarget diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f95802..2daff56 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 { virNodeAllocPages; } LIBVIRT_1.2.8; +LIBVIRT_1.2.10 { +global: +virDomainResumeFlags; +} LIBVIRT_1.2.9; + # define new API here using predicted next version number diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..3dc1c8f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8240,6 +8240,7 @@ static virHypervisorDriver hypervisor_driver = { .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ +.domainResumeFlags = remoteDomainResumeFlags, /* 1.2.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db12cda..dfd816e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -803,6 +803,11 @@ struct remote_domain_resume_args { remote_nonnull_domain dom;
[libvirt] [PATCH 4/5] qemuDomainResumeFlags: Introduce VIR_DOMAIN_RESUME_SYNC_TIME
This flag can be used to sync the domain's time right after domain CPUs are started. It's basically backing call of two subsequent APIs into one: 1) virDomainResume(dom) 2) virDomainSetTime(dom, 0, 0, VIR_DOMAIN_TIME_SYNC) Signed-off-by: Michal Privoznik --- include/libvirt/libvirt-domain.h | 4 src/qemu/qemu_driver.c | 7 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1795dd3..8d763c7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -867,6 +867,10 @@ int virDomainFree (virDomainPtr domain); /* * Domain suspend/resume */ +typedef enum { +VIR_DOMAIN_RESUME_SYNC_TIME = 1 << 0, /* on resume sync domain time from its RTC */ +} virDomainResumeFlagsValues; + int virDomainSuspend(virDomainPtr domain); int virDomainResume (virDomainPtr domain); int virDomainResumeFlags(virDomainPtr domain, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 903ca5d..38926c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1933,7 +1933,7 @@ qemuDomainResumeFlags(virDomainPtr dom, virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; -virCheckFlags(0, -1); +virCheckFlags(VIR_DOMAIN_RESUME_SYNC_TIME, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; @@ -1974,6 +1974,11 @@ qemuDomainResumeFlags(virDomainPtr dom, goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; + +if (flags & VIR_DOMAIN_RESUME_SYNC_TIME && +qemuDomainSetTimeImpl(driver, vm, 0, 0, true) < 0) +goto endjob; + ret = 0; endjob: -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Allow time sync on domain resume
*** BLURB HERE *** Michal Privoznik (5): Introduce virDomainResumeFlags API Implement virDomainResumeFlags in all drivers qemu: Split qemuDomainSetTime into two functions qemuDomainResumeFlags: Introduce VIR_DOMAIN_RESUME_SYNC_TIME virsh: Implement virDomainResumeFlags include/libvirt/libvirt-domain.h | 6 +++ src/driver-hypervisor.h | 5 ++ src/esx/esx_driver.c | 14 +- src/hyperv/hyperv_driver.c | 14 +- src/libvirt-domain.c | 44 src/libvirt_public.syms | 5 ++ src/libxl/libxl_driver.c | 14 +- src/lxc/lxc_driver.c | 15 +- src/openvz/openvz_driver.c | 13 - src/parallels/parallels_driver.c | 11 +++- src/phyp/phyp_driver.c | 12 - src/qemu/qemu_driver.c | 106 ++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 - src/remote_protocol-structs | 5 ++ src/test/test_driver.c | 13 - src/vbox/vbox_common.c | 11 +++- src/vmware/vmware_driver.c | 12 - src/xen/xen_driver.c | 14 +- src/xenapi/xenapi_driver.c | 21 +++- tools/virsh-domain.c | 13 - tools/virsh.pod | 10 ++-- 22 files changed, 316 insertions(+), 56 deletions(-) -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] Implement virDomainResumeFlags in all drivers
This practically boils down to: 1) rename DomainResume implementation to DomainResumeFlags 2) make DomainResume call DomainResumeFlags(dom, 0); Signed-off-by: Michal Privoznik --- src/esx/esx_driver.c | 14 +- src/hyperv/hyperv_driver.c | 14 +- src/libxl/libxl_driver.c | 14 -- src/lxc/lxc_driver.c | 15 +-- src/openvz/openvz_driver.c | 13 - src/parallels/parallels_driver.c | 11 ++- src/phyp/phyp_driver.c | 12 +++- src/qemu/qemu_driver.c | 15 +-- src/test/test_driver.c | 13 - src/vbox/vbox_common.c | 11 ++- src/vmware/vmware_driver.c | 12 +++- src/xen/xen_driver.c | 14 -- src/xenapi/xenapi_driver.c | 21 +++-- 13 files changed, 161 insertions(+), 18 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0770e89..aaca532 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1780,7 +1780,8 @@ esxDomainSuspend(virDomainPtr domain) static int -esxDomainResume(virDomainPtr domain) +esxDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1791,6 +1792,8 @@ esxDomainResume(virDomainPtr domain) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; +virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1838,6 +1841,14 @@ esxDomainResume(virDomainPtr domain) static int +esxDomainResume(virDomainPtr domain) +{ +return esxDomainResumeFlags(domain, 0); +} + + + +static int esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -5310,6 +5321,7 @@ static virHypervisorDriver esxDriver = { .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */ .domainSuspend = esxDomainSuspend, /* 0.7.0 */ .domainResume = esxDomainResume, /* 0.7.0 */ +.domainResumeFlags = esxDomainResumeFlags, /* 1.2.10 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 5ffcb85..ac8f745 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -569,12 +569,15 @@ hypervDomainSuspend(virDomainPtr domain) static int -hypervDomainResume(virDomainPtr domain) +hypervDomainResumeFlags(virDomainPtr domain, +unsigned int flags) { int result = -1; hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; +virCheckFlags(0, -1); + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { goto cleanup; } @@ -598,6 +601,14 @@ hypervDomainResume(virDomainPtr domain) static int +hypervDomainResume(virDomainPtr domain) +{ +return hypervDomainResumeFlags(domain, 0); +} + + + +static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -1370,6 +1381,7 @@ static virHypervisorDriver hypervDriver = { .domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */ .domainSuspend = hypervDomainSuspend, /* 0.9.5 */ .domainResume = hypervDomainResume, /* 0.9.5 */ +.domainResumeFlags = hypervDomainResumeFlags, /* 1.2.10 */ .domainDestroy = hypervDomainDestroy, /* 0.9.5 */ .domainDestroyFlags = hypervDomainDestroyFlags, /* 0.9.5 */ .domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d2c077c..342d0a5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -814,7 +814,8 @@ libxlDomainSuspend(virDomainPtr dom) static int -libxlDomainResume(virDomainPtr dom) +libxlDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); @@ -823,10 +824,12 @@ libxlDomainResume(virDomainPtr dom) virObjectEventPtr event = NULL; int ret = -1; +virCheckFlags(0, -1); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; -if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) +if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) @@ -873,6 +876,12 @@ libxlDomainResume(virDomainPtr dom) } static int +libxlDomainResume(virDomainPtr dom) +{ +return libxlDomainResumeFlags(dom, 0); +} + +static int libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virDomainObjPtr vm; @@ -4750,6 +4759,7 @@ static virHypervisorDriver libxlDriver = { .domainLookupByName
[libvirt] [PATCH 5/5] virsh: Implement virDomainResumeFlags
This is done by introducing '--sync-time' to already existing 'resume' command. If the option is used, the newer Flags() API is called. Signed-off-by: Michal Privoznik --- tools/virsh-domain.c | 13 - tools/virsh.pod | 10 ++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dfc3a8c..c02f0a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5135,6 +5135,10 @@ static const vshCmdOptDef opts_resume[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, +{.name = "sync-time", + .type = VSH_OT_BOOL, + .help = N_("sync domain's time from its RTC") +}, {.name = NULL} }; @@ -5144,11 +5148,18 @@ cmdResume(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; +unsigned int flags = 0; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; -if (virDomainResume(dom) == 0) { +if (vshCommandOptBool(cmd, "sync-time")) +flags |= VIR_DOMAIN_RESUME_SYNC_TIME; + +if ((flags && + virDomainResumeFlags(dom, flags) == 0) || +(!flags && + virDomainResume(dom) == 0)) { vshPrint(ctl, _("Domain %s resumed\n"), name); } else { vshError(ctl, _("Failed to resume domain %s"), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index daddb48..001f61a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2101,11 +2101,13 @@ is only supported with container based virtualization. Suspend a running domain. It is kept in memory but won't be scheduled anymore. -=item B I +=item B I [I<--sync-time>] -Moves a domain out of the suspended state. This will allow a previously -suspended domain to now be eligible for scheduling by the underlying -hypervisor. +Moves a domain out of the suspended state. This will allow a +previously suspended domain to now be eligible for scheduling by the +underlying hypervisor. Moreover, if I<--sync-time> is specified the +domain's time is synced right after the domain CPUs are started. Note, +that this may require guest agent on some hypervisors. =item B I I [I<--duration>] -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Local qemu migration
On Wed, Nov 05, 2014 at 01:16:08PM +0100, Marc-André Lureau wrote: > Hi > > On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange > wrote: > > > Nope, there's stuff that's on the filesystem that is tied to the QEMU > > process and that would clash when migrating as you have two copies of > > the same VM running at the sme time. > > > > Ok, there is no guarantee that the VMs won't write concurrently to the disk > during migration? > > Also, there are cases where VM disks/images are readonly, or image-less VMs. No, it isn't about the disk images. The actual guest CPUs are only executing in one QEMU at a time, so there's not a problem wrt disk image usage. The issue is with various other files we use. For example the QEMU monitor is a UNIX domain socket at a specific path - you can't have both QEMUs have the same UNIX domain socket. That's not under user control so we could change the monitor socket path, but the problem extends to stuff that is directly from the guest XML. ie VNC can be told to listen on a UNIX domain socket path. virtio-serial devices are typically bound to UNIX domain sockets. Serial ports, parallel ports, certain network device backends, and so on. Migration fundamentally requires that the two QEMU processes involved have completely separate filesystem namespaces, which effectively means separate hosts (or at least separate containers which is effectively the same thing) > Except the obvious testing case, there are other use cases that could be > interesting: moving a running VM from system to session libvirt, or the > other way around, or to a different user. Migrating from session to system libvirt or vica-verca isn't something that is reasonable to support either because of the different privilege levels. The session QEMU will require the disk images to be owned by one user account, the system QEMU will require them to be owned by a different user account. We can't chown the images to satisfy both the system and session QEMU at the same time. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Local qemu migration
Hi On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange wrote: > Nope, there's stuff that's on the filesystem that is tied to the QEMU > process and that would clash when migrating as you have two copies of > the same VM running at the sme time. > Ok, there is no guarantee that the VMs won't write concurrently to the disk during migration? Also, there are cases where VM disks/images are readonly, or image-less VMs. Except the obvious testing case, there are other use cases that could be interesting: moving a running VM from system to session libvirt, or the other way around, or to a different user. -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] remote: Fix memory leak in remoteConnectGetAllDomainStats
The remote call actually doesn't free the arguments array so we leak memory in case a domain list is specified. As the remote domain list array consists only of stolen pointers from the actual domain objects it's sufficient just to free the array. Valgrind message: ==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726 ==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618) ==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191) ==1081452==by 0x505D21E: remoteConnectGetAllDomainStats (remote_driver.c:7785) ==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080) ==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147) ==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935) ==1081452==by 0x133FEB: main (virsh.c:3719) --- src/remote/remote_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..d111e10 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, VIR_FREE(elem); } virDomainStatsRecordListFree(tmpret); +VIR_FREE(args.doms.doms_val); xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret, (char *) &ret); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Numa : Allow specification of 'units' for numa nodes.
>From 7fe8487c5e8d24086637d2157bad25322b3654f7 Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 07:53:59 +0530 CPU numa topology implicitly allows memory specification in 'KiB'. Enabling this to accept the 'unit' in which memory needs to be specified. This now allows users to specify memory in units of choice, and lists the same in 'KiB' -- just like other 'memory' elements in XML. I wanted to use virDomainParseScaledValue(), but that function implicitly assumes an XML layout where 'memory' is an _element_ of type 'scaledInteger', with 'unit' as its attribute. A NUMA cell has XML specification where 'memory' is an _attribute_ of element 'cell'. Since changing XML layout is not an option, I have borrowed code from the same. Looking forward to suggestions on how this can best be done. Signed-off-by: Prerna Saxena --- docs/schemas/domaincommon.rng | 5 + src/conf/cpu_conf.c | 36 ++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..44cabad 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4144,6 +4144,11 @@ + + + + + shared diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5475c07..65b9815 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -189,6 +189,7 @@ virCPUDefParseXML(xmlNodePtr node, char *cpuMode; char *fallback = NULL; char *vendor_id = NULL; +unsigned long long max; if (!xmlStrEqual(node->name, BAD_CAST "cpu")) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -439,10 +440,20 @@ virCPUDefParseXML(xmlNodePtr node, def->ncells = n; +/* 32 vs 64 bit will differ in value of upper memory bound. + * On 32-bit machines, our bound is 0x * KiB. On 64-bit + * machines, our bound is off_t (2^63). + */ +if (sizeof(unsigned long) < sizeof(long long)) +max = 1024ull * ULONG_MAX; +else +max = LLONG_MAX; + for (i = 0; i < n; i++) { -char *cpus, *memory, *memAccessStr; +char *cpus, *memory, *memAccessStr, *unit; int ret, ncpus = 0; unsigned int cur_cell; +unsigned long long bytes; char *tmp = NULL; tmp = virXMLPropString(nodes[i], "id"); @@ -496,14 +507,34 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &bytes); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); VIR_FREE(memory); goto error; } + +unit = virXMLPropString(nodes[i], "unit"); +if (!unit) { +if (VIR_STRDUP(unit, "KiB") < 0) +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error processing 'memory' in NUMA cell")); +} + +if (virScaleInteger(&bytes, unit, 1024, max) < 0) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("Error scaling 'memory' in NUMA cell")); +VIR_FREE(memory); +VIR_FREE(unit); +goto error; +} + +def->cells[cur_cell].mem = VIR_DIV_UP(bytes, 1024); + +bytes = 0; VIR_FREE(memory); +VIR_FREE(unit); memAccessStr = virXMLPropString(nodes[i], "memAccess"); if (memAccessStr) { @@ -703,6 +734,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAsprintf(buf, " id='%zu'", i); virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr); virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); +virBufferAddLit(buf, " unit='KiB'"); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3]Test:Augment test cases to correctly model NUMA specification.
>From 2fe0e329e7224b7cd29e1252e4b4e70d9195ab2b Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Mon, 3 Nov 2014 15:16:12 +0530 This adds the tag 'unit="KiB"' for memory attribute in NUMA cells. Signed-off-by: Prerna Saxena --- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml| 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml | 8 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml | 6 +++--- .../qemuxml2argv-numatune-memnodes-problematic.xml| 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml | 6 +++--- 18 files changed, 42 insertions(+), 42 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml index 474a238..bdffcd1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml index cf7c040..c638ffa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml index 0543f7f..20120e9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml index 0a5f9fc..a90e7a2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml index fa3070d..ea2dc81 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml @@ -11,8 +11,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml index 5ad0695..b67df2f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml @@ -20,10 +20,10 @@ - - - - + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml index 3df870b..6afa6ef 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml @@ -15,8 +15,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml index 35aa2cf..21f4985 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml @@ -15,8 +15,8 @@ - - + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml index a3ed29b..eb18f24 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml @@ -20,10 +20,10 @@ - - - - + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hug
[libvirt] [PATCH 1/3] Memory: Use consistent type for all memory elements.
>From 4b3e336ea045759758b04440d75802e990506e2b Mon Sep 17 00:00:00 2001 From: Prerna Saxena Date: Fri, 31 Oct 2014 16:07:21 +0530 Domain memory elements such as max_balloon and cur_balloon are implemented as 'unsigned long long', whereas the 'memory' element in NUMA cells is implemented as 'unsigned int'. Use the same data type (unsigned long long) for 'memory' element in NUMA cells. Signed-off-by: Prerna Saxena --- src/conf/cpu_conf.c | 4 ++-- src/conf/cpu_conf.h | 2 +- src/qemu/qemu_command.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 9b7fbb0..5475c07 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -496,7 +496,7 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } -ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); +ret = virStrToLong_ull(memory, NULL, 10, &def->cells[cur_cell].mem); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); @@ -702,7 +702,7 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "cells[i].cpustr); -virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem); +virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem); if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virMemAccessTypeToString(memAccess)); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d45210b..5bcf101 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -105,7 +105,7 @@ typedef virCellDef *virCellDefPtr; struct _virCellDef { virBitmapPtr cpumask; /* CPUs that are part of this node */ char *cpustr; /* CPUs stored in string form for dumpxml */ -unsigned int mem; /* Node memory in kB */ +unsigned long long mem; /* Node memory in kB */ virMemAccess memAccess; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..13b54dd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6693,7 +6693,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, } for (i = 0; i < def->cpu->ncells; i++) { -int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); +unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; virMemAccess memAccess = def->cpu->cells[i].memAccess; @@ -6799,7 +6799,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAddLit(&buf, "memory-backend-ram"); } -virBufferAsprintf(&buf, ",size=%dM,id=ram-node%zu", cellmem, i); +virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i); if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset, &nodemask, i) < 0) @@ -6849,7 +6849,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); } else { -virBufferAsprintf(&buf, ",mem=%d", cellmem); +virBufferAsprintf(&buf, ",mem=%llu", cellmem); } virCommandAddArgBuffer(cmd, &buf); -- 1.9.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes
This patch set addresses a bunch of memory & NUMA fixes. Series Description: === Patch 1/3 : Use consistent data type to represent memory elements in various XML attributes. This ensures all memory elements are always represented as 'unsigned long long'. Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of a NUMA cell. This enables users to easily describe how much memory needs to be allocated to each NUMA cell for a guest domain. Patch 3/3 : This augments test cases to add the 'unit' tag. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 0/2] v1.1 bugfix: support dhcp network interfaces
On 31.10.2014 19:02, Gene Czarcinski wrote: Resubmitted to add "sandbox PATCH" to Subject v1.1 adds some documentation changes. Support for a network such as -N dhcp,source=default was not working in that dhclient was not being started. Although I am not sure what the real problem is, the solution is to use g_spawn_sync() instead of g_spawn_async() to start /sbin/dhclient. The second patch addes "-v" to the dhclient arguments to improve debugging info. The dhclient into will be in /var/log/messages the Secure Contrainer host system and not in the container itself. Gene Czarcinski (2): v1.1 for dhclient use g_spawn_sync() v1.1 add -v to dhclient parameter arguments libvirt-sandbox/libvirt-sandbox-init-common.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) Even though this is not rebased as I asked (I still get conflicts when trying to apply the patches), I've resolved the conflicts, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-sandbox][PATCH 0/4] Couple of fixes to compile again
On Tue, Nov 04, 2014 at 03:40:47PM +0100, Michal Privoznik wrote: It's been a while that I tried to build libvirt-sandbox. And guess what, it doesn't compile cleanly so here are some patches to fix the issues I met. Michal Privoznik (4): virt-selinux.m4: Define SELINUX variables Makefile: link SELINUX into libvirt-sandbox-1.0.so m4: sync macros with libvirt libvirt-sandbox-config.c: Fix comment libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-config.c | 2 +- m4/manywarnings.m4 | 217 --- m4/virt-compile-warnings.m4 | 216 ++ m4/virt-selinux.m4 | 6 +- m4/warnings.m4 | 82 +--- 6 files changed, 369 insertions(+), 158 deletions(-) ACK series except 3/4 (read the response to that particular one). Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-sandbox][PATCH 3/4] m4: sync macros with libvirt
On Tue, Nov 04, 2014 at 03:40:50PM +0100, Michal Privoznik wrote: The macros under the m4 directory are outdated a bit. When trying to compile with newer gcc I see some errors: make[4]: Entering directory '/home/zippy/work/libvirt/libvirt-sanbox.git/libvirt-sandbox' CC libvirt_sandbox_1_0_la-libvirt-sandbox-main.lo gcc: warning: switch '-Wmudflap' is no longer supported Signed-off-by: Michal Privoznik --- m4/manywarnings.m4 | 217 m4/virt-compile-warnings.m4 | 216 +++ m4/warnings.m4 | 82 + 3 files changed, 361 insertions(+), 154 deletions(-) [...] diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 894ab59..532a777 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -2,7 +2,7 @@ dnl dnl Enable all known GCC compiler warnings, except for those dnl we can't yet cope with dnl -AC_DEFUN([LIBVIRT_SANDBOX_COMPILE_WARNINGS],[ +AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ This function changed its name due to the overwrite and ./configure now yells an error, but it doesn't fail, so it's easy to miss. apart from this particular file, which is specific for libvirt-related projects, it's ok to sync the rest with gnulib, as it's being done on regular (or rather irregular) basis. So partial ACK for m4/manywarnings.m4 and m4/warnings.m4 only. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Require at least one console for LXC domain
> -Original Message- > From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] > On Behalf Of Martin Kletzander > Sent: Wednesday, November 05, 2014 4:10 PM > To: Ján Tomko > Cc: libvir-list@redhat.com [snip] > > > >It's been broken long enough to make me think nobody cares about this > >functionality, but I could make it work if you think it's worthwhile. > > > > Not worth while, I agree with you. It can be done later on if someone > really wants it. > > ACK, > Agree. But we could do some improvements such as adding a console for containers automatically if we do not provided one in XML. Thanks - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Require at least one console for LXC domain
On Tue, Nov 04, 2014 at 03:21:53PM +0100, Ján Tomko wrote: On 10/31/2014 11:53 AM, Martin Kletzander wrote: On Fri, Oct 31, 2014 at 10:22:25AM +0100, Ján Tomko wrote: A domain without a console quietly dies soon after start, because we try to set /dev/null as a controlling TTY 2014-10-30 15:10:59.705+: 1: error : lxcContainerSetupFDs:283 : ioctl(TIOCSTTY) failed: Inappropriate ioctl for device s/TIOCSTTY/TIOCSCTTY/ would make it more greppable (is that even a word?) and it's now true since you pushed your trivial fix for that ;) I have amended the commit message. Report an error early instead of trying to start it. https://bugzilla.redhat.com/show_bug.cgi?id=1155410 --- src/lxc/lxc_container.c | 6 -- src/lxc/lxc_process.c | 6 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index f02b959..8aba3ba 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2093,8 +2093,10 @@ static int lxcContainerChild(void *data) if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", LXC_STATE_DIR, vmDef->name, tty) < 0) goto cleanup; -} else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) { -goto cleanup; +} else { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("At least one tty is required")); +goto cleanup; } How about we don't attach any tty and use the ioctl(TIOCSTTY) to steal controlling terminal (or call open() with O_NOCTTY, or just don't call ioctl() at all, I don't know what we normally use)? Then there would be no controlling terminal, but there would still be a session leader, wouldn't it? I don't know if we want to run a container without a controlling terminal. Looking at git history, back when we only supported one console and none was specified, it seems we quietly opened one anyway and didn't report it in the XML. Alternatively, we could auto-add a console in the XML. Anyway, this version looks OK too since it doesn't break any use case (there is nothing to break now). ACK after release if you argue why not to use the idea above. It's been broken long enough to make me think nobody cares about this functionality, but I could make it work if you think it's worthwhile. Not worth while, I agree with you. It can be done later on if someone really wants it. ACK, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Local qemu migration
On Tue, Nov 04, 2014 at 10:49:44AM -0500, Cole Robinson wrote: > On 11/04/2014 10:43 AM, Marc-André Lureau wrote: > >Hi, > > > >Attempting to migration from session to system qemu fails because of the > >following checks in qemuMigrationCookieXMLParse(): > > > >if (STREQ(mig->remoteHostname, mig->localHostname)) { > > virReportError(VIR_ERR_INTERNAL_ERROR, > >_("Attempt to migrate guest to the same host %s"), > >mig->remoteHostname); > > goto error; > > } > > > > if (memcmp(mig->remoteHostuuid, mig->localHostuuid, VIR_UUID_BUFLEN) == > > 0) { > > virReportError(VIR_ERR_INTERNAL_ERROR, > >_("Attempt to migrate guest to the same host %s"), > >tmp); > > goto error; > > > >Is there a technical limitation for this error? If not, could it be overriden > >with an additional flag? > > Trying to prevent migrating to the same libvirtd instance which would > historically deadlock in libvirt's qemu driver. But migrating session -> > system shouldn't suffer from that problem, so maybe it should be smarter. > > Maybe with the more fine grained locking these days we can actually make > localhost migration work? Would be useful for testing Nope, there's stuff that's on the filesystem that is tied to the QEMU process and that would clash when migrating as you have two copies of the same VM running at the sme time. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't setup cpuset.mems if memory mode in numatune is 'preferred'
On Wed, Nov 05, 2014 at 12:00:14PM +0800, Wang Rui wrote: On 2014/11/4 22:04, Martin Kletzander wrote: On Tue, Nov 04, 2014 at 09:22:22PM +0800, Wang Rui wrote: If the memory mode is specified as preferred, we get the following error when starting domain. error: Unable to write to '$my_cgroup_path/cpuset.mems': Device or resource busy XML is configured with numatune as follows: If memory mode is 'preferred', cpuset.mems in cgroup shouldn't be set to 'nodeset'. I find that maybe commit 1a7be8c600905aa07ac2d78293336ba8523ad48e changes the former logic of checking mode in virDomainNumatuneGetNodeset. Signed-off-by: Wang Rui --- src/qemu/qemu_cgroup.c | 5 + 1 file changed, 5 insertions(+) Thanks for catching that, it definitely is a problem, but I think it is cause by commit 93e82727ec11d471d2ef3a18835e1fdfe062cef1. It should be also fixed in virLXCCgroupSetupCpusetTune() for LXC. OK. I'll try to fix it for LXC in another patch. Yeah, that can be a follow-up, it'll look the same, anyway. diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index b5bdb36..8685d6f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -618,6 +618,11 @@ qemuSetupCpusetMems(virDomainObjPtr vm, if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; +if (virDomainNumatuneGetMode(vm->def->numatune, -1) != +VIR_DOMAIN_NUMATUNE_MEM_STRICT) { +return 0; +} + One question, is it problem only for 'preferred' or 'interleaved' as well? Because if it's only problem for 'preferred', then the check is wrong. If it's problem for 'interleaved' as well, then the commit message is wrong. 'interleave' with a single node(such as nodeset='0') will cause the same error. But 'interleave' mode should not live with a single node. So maybe there's another bugfix to check 'interleave' with single node. Well, I'd be OK with just changing the commit message to mention that. This fix is still a valid one and will fix both issues, won't it? If configured with 'interleave' and multiple nodes(such as nodeset='0-1'), VM can be started successfully. And cpuset.mems is set to the same nodeset. So I'll revise my patch. I'll send patches V2. Conclusion: 1/3 : add check for 'interleave' mode with single numa node 2/3 : fix this problem in qemu 3/3 : fix this problem in lxc Is it OK? Anyway, after either one is fixed, I can push this. Thank you, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list