Re: [libvirt] [PATCH v2] lxc: Refresh capabilities if they have never been initalized
On Mon, Dec 09, 2019 at 02:58:51PM -0500, Cole Robinson wrote: > Adjust virLXCDriverGetCapabilities to fill in driver->caps if it is > empty, regardless of the passed 'refresh' value. This matches the > pattern used in virQEMUDriverGetCapabilities > > This fixes LXC XML startup parsing for me > > Signed-off-by: Cole Robinson > --- > v2: > Use the virQEMUDriverGetCapabilities like danpb suggested > > src/lxc/lxc_conf.c | 8 > 1 file changed, 8 insertions(+) Reviewed-by: Daniel P. Berrangé > diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c > index 2df1537b22..adf7a0b66c 100644 > --- a/src/lxc/lxc_conf.c > +++ b/src/lxc/lxc_conf.c > @@ -196,6 +196,14 @@ virCapsPtr virLXCDriverGetCapabilities(virLXCDriverPtr > driver, > driver->caps = caps; > } else { > lxcDriverLock(driver); > + > +if (driver->caps == NULL || > +driver->caps->nguests == 0) { nitpick: nguests will never be zero in LXC since we hardcode the set of guests, so you can drop that part of the condition when pushing. > +VIR_DEBUG("Capabilities didn't detect any guests. Forcing a " > + "refresh."); > +lxcDriverUnlock(driver); > +return virLXCDriverGetCapabilities(driver, true); > +} > } > > ret = virObjectRef(driver->caps); Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: Refresh capabilities if they have never been initalized
Adjust virLXCDriverGetCapabilities to fill in driver->caps if it is empty, regardless of the passed 'refresh' value. This matches the pattern used in virQEMUDriverGetCapabilities This fixes LXC XML startup parsing for me Signed-off-by: Cole Robinson --- v2: Use the virQEMUDriverGetCapabilities like danpb suggested src/lxc/lxc_conf.c | 8 1 file changed, 8 insertions(+) diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 2df1537b22..adf7a0b66c 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -196,6 +196,14 @@ virCapsPtr virLXCDriverGetCapabilities(virLXCDriverPtr driver, driver->caps = caps; } else { lxcDriverLock(driver); + +if (driver->caps == NULL || +driver->caps->nguests == 0) { +VIR_DEBUG("Capabilities didn't detect any guests. Forcing a " + "refresh."); +lxcDriverUnlock(driver); +return virLXCDriverGetCapabilities(driver, true); +} } ret = virObjectRef(driver->caps); -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] src: lxc: Fix typo in a Makefile variable
In commit 0985a9597bb0348d46c0d18dc548a676bf0ad8e2 we stopped distributing generated source file. This is done by prepending binary_SOURCES variable with "nodist_". However, there is a typo - the prefix is "nodst_" instead of "nodist_". Signed-off-by: Michal Privoznik --- Pushed under trivial rule. src/lxc/Makefile.inc.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/Makefile.inc.am b/src/lxc/Makefile.inc.am index d3a7cbf93b..6aaec09fb4 100644 --- a/src/lxc/Makefile.inc.am +++ b/src/lxc/Makefile.inc.am @@ -127,7 +127,7 @@ augeastest_DATA += lxc/test_virtlxcd.aug CLEANFILES += lxc/virtlxcd.aug virtlxcd_SOURCES = $(REMOTE_DAEMON_SOURCES) -nodst_virtlxcd_SOURCES = $(REMOTE_DAEMON_GENERATED) +nodist_virtlxcd_SOURCES = $(REMOTE_DAEMON_GENERATED) virtlxcd_CFLAGS = \ $(REMOTE_DAEMON_CFLAGS) \ -DDAEMON_NAME="\"virtlxcd\"" \ -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Revert "lxc: Try harder to stop/reboot containers"
This reverts commit 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c. If virLXCDomainSetRunlevel returns -1 this indicates a serious error / failure that must be propagated to the caller. We must not carry on with other shutdown methods in this case. If virLXCDomainSetRunlevel return 0, this indicates that no initctl was found and it is thus reasonable to fallback to sending SIGTERM. The commit being reverted is broken because it would fallback to SIGTERM when virLXCDomainSetRunlevel returns -1, and would not fallback when virLXCDomainSetRunlevel returns 0. ie it did the exact opposite of what was required. Signed-off-by: Daniel P. Berrangé --- src/lxc/lxc_driver.c | 40 ++-- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9db2a02dee..01af5f6db8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3262,7 +3262,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; -int rc = -1; +int rc; virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL | VIR_DOMAIN_SHUTDOWN_SIGNAL, -1); @@ -3291,17 +3291,19 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; -if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { -if (flags != 0 && -(flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); -goto endjob; -} +if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) +goto endjob; +if (rc == 0 && flags != 0 && +((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); +goto endjob; } +} else { +rc = 0; } -if (rc < 0 && +if (rc == 0 && (flags == 0 || (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { if (kill(priv->initpid, SIGTERM) < 0 && @@ -3338,7 +3340,7 @@ lxcDomainReboot(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; -int rc = -1; +int rc; virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL | VIR_DOMAIN_REBOOT_SIGNAL, -1); @@ -3367,17 +3369,19 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; -if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { -if (flags != 0 && -(flags & VIR_DOMAIN_REBOOT_INITCTL)) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); -goto endjob; -} +if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) +goto endjob; +if (rc == 0 && flags != 0 && +((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); +goto endjob; } +} else { +rc = 0; } -if (rc < 0 && +if (rc == 0 && (flags == 0 || (flags & VIR_DOMAIN_REBOOT_SIGNAL))) { if (kill(priv->initpid, SIGHUP) < 0 && -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
On 11/11/18 12:46 PM, Julio Faracco wrote: > Em sáb, 10 de nov de 2018 às 11:17, John Ferlan escreveu: >> >> >> >> On 11/9/18 12:30 PM, Julio Faracco wrote: >>> This patch introduce the new settings for LXC 3.0 or higher. The older >>> versions keep the compatibility to deprecated settings for LXC, but >>> after release 3.0, the compatibility was removed. This commit adds the >>> support to the refactored settings. >>> >>> v1-v2: Michal's suggestions to handle differences between versions. >>> v2-v3: Adding suggestions from Pino and John too. >> >> These type of comments would go below the --- below so that they're not >> part of commit history... >> >>> >>> Signed-off-by: Julio Faracco >>> --- >>> src/lxc/lxc_native.c | 45 +++- >>> 1 file changed, 32 insertions(+), 13 deletions(-) >>> >>> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c >>> index cbdec723dd..d3ba12bb0e 100644 >>> --- a/src/lxc/lxc_native.c >>> +++ b/src/lxc/lxc_native.c >> >> [...] >> >>> @@ -724,12 +734,13 @@ lxcIdmapWalkCallback(const char *name, >>> virConfValuePtr value, void *data) >>> char type; >>> unsigned long start, target, count; >>> >>> -if (STRNEQ(name, "lxc.id_map") || !value->str) >>> +/* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ >>> +if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") || >>> !value->str) >>> return 0; >> >> This one caused lxcconf2xmltest to fail and needs to change to: >> >> /* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ >> if (STRNEQ(name, "lxc.idmap") || !value->str) { >> if (!value->str || STRNEQ(name, "lxc.id_map")) >> return 0; >> } >> >> The failure occurred because of the STRNEQ OR not being true (silly me >> on first pass not running the tests too ;-)) >> >>> >>> if (sscanf(value->str, "%c %lu %lu %lu", , >>> , , ) != 4) { >>> -virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid lxc.id_map: >>> '%s'"), >>> +virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid: '%s'"), >> >> Do you mind if I alter this to: >> >> virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid %s: '%s'"), >>name, value->str); >> >> That way the conf name string is provided like it was before >> >> >>> value->str); >>> return -1; >>> } >>> @@ -1041,19 +1052,27 @@ lxcParseConfigString(const char *config, >>> if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) >>> goto error; >>> >>> -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || >>> -VIR_STRDUP(vmdef->name, value) < 0) >>> -goto error; >>> +if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) { >>> +virResetLastError(); >>> + >>> +/* Check for pre LXC 3.0 legacy key */ >>> +if (virConfGetValueString(properties, "lxc.utsname", ) <= 0) >>> +goto error; >>> +} >>> + >> >> I think in this case the @value needs to be restored... Previous if the >> GetValueString OR the VIR_STRDUP of the value was < 0, we go to error. >> Although I'm not quite sure how @value would be NULL so as to cause the >> subsequent line to be executed... In any case, copying @value needs to >> be done, so add: >> >> if (VIR_STRDUP(vmdef->name, value) < 0) >> goto error; >> >> Which I can add if you agree. > > No problems, John. You can go ahead with the changes. > I forgot too add VIR_STRDUP after checking the property. > It was causing the test error. > >> >> With those changes, >> >> Reviewed-by: John Ferlan >> >> John >> >> As a follow-up maybe adding/altering/updating the tests/lxcconf2xmldata >> to include both pre and post 3.0 type data would be a good thing. > > Yes, I agree too. But only config files that don't have netowork settings. > Version 3.X and higher have another syntax to configure network too. > And it was not implemented yet. I'm planning to propose this feature > in the future. > >> >> [...] Since you have access to the V3.0 environment, perhaps it's best that you update the patch based on my comments and also add the test *.config files using the v3 syntax. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
On 11/12/18 6:13 AM, Daniel P. Berrangé wrote: > On Fri, Nov 09, 2018 at 03:30:59PM -0200, Julio Faracco wrote: >> This patch introduce the new settings for LXC 3.0 or higher. The older >> versions keep the compatibility to deprecated settings for LXC, but >> after release 3.0, the compatibility was removed. This commit adds the >> support to the refactored settings. >> >> v1-v2: Michal's suggestions to handle differences between versions. >> v2-v3: Adding suggestions from Pino and John too. >> >> Signed-off-by: Julio Faracco >> --- >> src/lxc/lxc_native.c | 45 +++- >> 1 file changed, 32 insertions(+), 13 deletions(-) > I'd expect to additions to the test suite to cover these changes > eg lxcconf2xmltest data files Actually, I was just going to send mail saying that this patch breaks the *existing* lxcfonc2xml tests. (run "make check" and you'll see the failure, then run "VIR_TEST_DEBUG=1 tests/lxcconf2xml" to see more details about the failure - it's getting the name of the new domain wrong) pEpkey.asc Description: application/pgp-keys -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
On Fri, Nov 09, 2018 at 03:30:59PM -0200, Julio Faracco wrote: > This patch introduce the new settings for LXC 3.0 or higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. > > v1-v2: Michal's suggestions to handle differences between versions. > v2-v3: Adding suggestions from Pino and John too. > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_native.c | 45 +++- > 1 file changed, 32 insertions(+), 13 deletions(-) I'd expect to additions to the test suite to cover these changes eg lxcconf2xmltest data files Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
Em sáb, 10 de nov de 2018 às 11:17, John Ferlan escreveu: > > > > On 11/9/18 12:30 PM, Julio Faracco wrote: > > This patch introduce the new settings for LXC 3.0 or higher. The older > > versions keep the compatibility to deprecated settings for LXC, but > > after release 3.0, the compatibility was removed. This commit adds the > > support to the refactored settings. > > > > v1-v2: Michal's suggestions to handle differences between versions. > > v2-v3: Adding suggestions from Pino and John too. > > These type of comments would go below the --- below so that they're not > part of commit history... > > > > > Signed-off-by: Julio Faracco > > --- > > src/lxc/lxc_native.c | 45 +++- > > 1 file changed, 32 insertions(+), 13 deletions(-) > > > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > > index cbdec723dd..d3ba12bb0e 100644 > > --- a/src/lxc/lxc_native.c > > +++ b/src/lxc/lxc_native.c > > [...] > > > @@ -724,12 +734,13 @@ lxcIdmapWalkCallback(const char *name, > > virConfValuePtr value, void *data) > > char type; > > unsigned long start, target, count; > > > > -if (STRNEQ(name, "lxc.id_map") || !value->str) > > +/* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ > > +if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") || > > !value->str) > > return 0; > > This one caused lxcconf2xmltest to fail and needs to change to: > > /* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ > if (STRNEQ(name, "lxc.idmap") || !value->str) { > if (!value->str || STRNEQ(name, "lxc.id_map")) > return 0; > } > > The failure occurred because of the STRNEQ OR not being true (silly me > on first pass not running the tests too ;-)) > > > > > if (sscanf(value->str, "%c %lu %lu %lu", , > > , , ) != 4) { > > -virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid lxc.id_map: > > '%s'"), > > +virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid: '%s'"), > > Do you mind if I alter this to: > > virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid %s: '%s'"), >name, value->str); > > That way the conf name string is provided like it was before > > > > value->str); > > return -1; > > } > > @@ -1041,19 +1052,27 @@ lxcParseConfigString(const char *config, > > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) > > goto error; > > > > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || > > -VIR_STRDUP(vmdef->name, value) < 0) > > -goto error; > > +if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) { > > +virResetLastError(); > > + > > +/* Check for pre LXC 3.0 legacy key */ > > +if (virConfGetValueString(properties, "lxc.utsname", ) <= 0) > > +goto error; > > +} > > + > > I think in this case the @value needs to be restored... Previous if the > GetValueString OR the VIR_STRDUP of the value was < 0, we go to error. > Although I'm not quite sure how @value would be NULL so as to cause the > subsequent line to be executed... In any case, copying @value needs to > be done, so add: > > if (VIR_STRDUP(vmdef->name, value) < 0) > goto error; > > Which I can add if you agree. No problems, John. You can go ahead with the changes. I forgot too add VIR_STRDUP after checking the property. It was causing the test error. > > With those changes, > > Reviewed-by: John Ferlan > > John > > As a follow-up maybe adding/altering/updating the tests/lxcconf2xmldata > to include both pre and post 3.0 type data would be a good thing. Yes, I agree too. But only config files that don't have netowork settings. Version 3.X and higher have another syntax to configure network too. And it was not implemented yet. I'm planning to propose this feature in the future. > > [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
On 11/9/18 12:30 PM, Julio Faracco wrote: > This patch introduce the new settings for LXC 3.0 or higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. > > v1-v2: Michal's suggestions to handle differences between versions. > v2-v3: Adding suggestions from Pino and John too. These type of comments would go below the --- below so that they're not part of commit history... > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_native.c | 45 +++- > 1 file changed, 32 insertions(+), 13 deletions(-) > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > index cbdec723dd..d3ba12bb0e 100644 > --- a/src/lxc/lxc_native.c > +++ b/src/lxc/lxc_native.c [...] > @@ -724,12 +734,13 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr > value, void *data) > char type; > unsigned long start, target, count; > > -if (STRNEQ(name, "lxc.id_map") || !value->str) > +/* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ > +if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") || > !value->str) > return 0; This one caused lxcconf2xmltest to fail and needs to change to: /* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ if (STRNEQ(name, "lxc.idmap") || !value->str) { if (!value->str || STRNEQ(name, "lxc.id_map")) return 0; } The failure occurred because of the STRNEQ OR not being true (silly me on first pass not running the tests too ;-)) > > if (sscanf(value->str, "%c %lu %lu %lu", , > , , ) != 4) { > -virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid lxc.id_map: '%s'"), > +virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid: '%s'"), Do you mind if I alter this to: virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid %s: '%s'"), name, value->str); That way the conf name string is provided like it was before > value->str); > return -1; > } > @@ -1041,19 +1052,27 @@ lxcParseConfigString(const char *config, > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) > goto error; > > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || > -VIR_STRDUP(vmdef->name, value) < 0) > -goto error; > +if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) { > +virResetLastError(); > + > +/* Check for pre LXC 3.0 legacy key */ > +if (virConfGetValueString(properties, "lxc.utsname", ) <= 0) > +goto error; > +} > + I think in this case the @value needs to be restored... Previous if the GetValueString OR the VIR_STRDUP of the value was < 0, we go to error. Although I'm not quite sure how @value would be NULL so as to cause the subsequent line to be executed... In any case, copying @value needs to be done, so add: if (VIR_STRDUP(vmdef->name, value) < 0) goto error; Which I can add if you agree. With those changes, Reviewed-by: John Ferlan John As a follow-up maybe adding/altering/updating the tests/lxcconf2xmldata to include both pre and post 3.0 type data would be a good thing. [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] lxc: Include support to lxc version 3.0 or higher.
This patch introduce the new settings for LXC 3.0 or higher. The older versions keep the compatibility to deprecated settings for LXC, but after release 3.0, the compatibility was removed. This commit adds the support to the refactored settings. v1-v2: Michal's suggestions to handle differences between versions. v2-v3: Adding suggestions from Pino and John too. Signed-off-by: Julio Faracco --- src/lxc/lxc_native.c | 45 +++- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index cbdec723dd..d3ba12bb0e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -200,8 +200,13 @@ lxcSetRootfs(virDomainDefPtr def, int type = VIR_DOMAIN_FS_TYPE_MOUNT; VIR_AUTOFREE(char *) value = NULL; -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) -return -1; +if (virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) { +virResetLastError(); + +/* Check for pre LXC 3.0 legacy key */ +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) +return -1; +} if (STRPREFIX(value, "/dev/")) type = VIR_DOMAIN_FS_TYPE_BLOCK; @@ -684,8 +689,13 @@ lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties) virDomainChrDefPtr console; size_t i; -if (virConfGetValueString(properties, "lxc.tty", ) <= 0) -return 0; +if (virConfGetValueString(properties, "lxc.tty.max", ) <= 0) { +virResetLastError(); + +/* Check for pre LXC 3.0 legacy key */ +if (virConfGetValueString(properties, "lxc.tty", ) <= 0) +return 0; +} if (virStrToLong_i(value, NULL, 10, ) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse int: '%s'"), @@ -724,12 +734,13 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr value, void *data) char type; unsigned long start, target, count; -if (STRNEQ(name, "lxc.id_map") || !value->str) +/* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ +if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") || !value->str) return 0; if (sscanf(value->str, "%c %lu %lu %lu", , , , ) != 4) { -virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid lxc.id_map: '%s'"), +virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid: '%s'"), value->str); return -1; } @@ -1041,19 +1052,27 @@ lxcParseConfigString(const char *config, if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) goto error; -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || -VIR_STRDUP(vmdef->name, value) < 0) -goto error; +if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) { +virResetLastError(); + +/* Check for pre LXC 3.0 legacy key */ +if (virConfGetValueString(properties, "lxc.utsname", ) <= 0) +goto error; +} + if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) goto error; if (lxcSetRootfs(vmdef, properties) < 0) goto error; -/* Look for fstab: we shouldn't have it */ -if (virConfGetValue(properties, "lxc.mount")) { +/* LXC 3.0 uses "lxc.mount.fstab", while legacy used just "lxc.mount". + * In either case, generate the error to use "lxc.mount.entry" instead */ +if (virConfGetValue(properties, "lxc.mount.fstab") || +virConfGetValue(properties, "lxc.mount")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("lxc.mount found, use lxc.mount.entry lines instead")); + _("lxc.mount.fstab or lxc.mount found, use " + "lxc.mount.entry lines instead")); goto error; } @@ -1069,7 +1088,7 @@ lxcParseConfigString(const char *config, if (lxcCreateConsoles(vmdef, properties) < 0) goto error; -/* lxc.id_map */ +/* lxc.idmap or legacy lxc.id_map */ if (virConfWalk(properties, lxcIdmapWalkCallback, vmdef) < 0) goto error; -- 2.19.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: Clang is complaining about possible NULL pointer.
On Wed, Nov 07, 2018 at 04:23:47PM -0500, John Ferlan wrote: On 11/7/18 3:57 PM, Julio Faracco wrote: The array "mount" inside lxc_container is not being checked before for loop. Clang syntax scan is complaining about this segmentation fault. Signed-off-by: Julio Faracco --- src/lxc/lxc_container.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) Reviewed-by: John Ferlan (and pushed) John FWIW: Ironically Coverity never complained about this one even though it's in the category of things Coverity doesn't like either ;-) My guess is that coverity was clever enough to know that thing can never happen (it could happen only if nmounts is non-zero and mounts is NULL). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: Clang is complaining about possible NULL pointer.
On 11/7/18 3:57 PM, Julio Faracco wrote: > The array "mount" inside lxc_container is not being checked before for > loop. Clang syntax scan is complaining about this segmentation fault. > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_container.c | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > Reviewed-by: John Ferlan (and pushed) John FWIW: Ironically Coverity never complained about this one even though it's in the category of things Coverity doesn't like either ;-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: Clang is complaining about possible NULL pointer.
The array "mount" inside lxc_container is not being checked before for loop. Clang syntax scan is complaining about this segmentation fault. Signed-off-by: Julio Faracco --- src/lxc/lxc_container.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 918194dacd..d834bf01d7 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -867,9 +867,13 @@ static int lxcContainerSetReadOnly(void) } } -if (mounts) -qsort(mounts, nmounts, sizeof(mounts[0]), - virStringSortRevCompare); +if (!mounts) { +ret = 0; +goto cleanup; +} + +qsort(mounts, nmounts, sizeof(mounts[0]), + virStringSortRevCompare); for (i = 0; i < nmounts; i++) { VIR_DEBUG("Bind readonly %s", mounts[i]); @@ -883,9 +887,7 @@ static int lxcContainerSetReadOnly(void) ret = 0; cleanup: -for (i = 0; i < nmounts; i++) -VIR_FREE(mounts[i]); -VIR_FREE(mounts); +virStringListFreeCount(mounts, nmounts); endmntent(procmnt); return ret; -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: Include support to lxc version 3.0 or higher.
On 11/5/18 1:57 PM, Julio Faracco wrote: > This patch introduce the new settings for LXC 3.0 or higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_native.c | 18 +- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > index cbdec723dd..8604cbaf46 100644 > --- a/src/lxc/lxc_native.c > +++ b/src/lxc/lxc_native.c > @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, > int type = VIR_DOMAIN_FS_TYPE_MOUNT; > VIR_AUTOFREE(char *) value = NULL; > > -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) > +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && > +/* Legacy config keys were removed after release 3.0. */ > +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) > return -1; So "integrating" Pino's v2 comment and Michal's RFC/v1 comment plus some error cleansing: if (virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) { virResetLastError(); /* Check for pre LXC 3.0 legacy key */ if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) return -1; } > > if (STRPREFIX(value, "/dev/")) > @@ -684,7 +686,9 @@ lxcCreateConsoles(virDomainDefPtr def, virConfPtr > properties) > virDomainChrDefPtr console; > size_t i; > > -if (virConfGetValueString(properties, "lxc.tty", ) <= 0) > +if (virConfGetValueString(properties, "lxc.tty", ) <= 0 && > +/* Legacy config keys were removed after release 3.0. */ > +virConfGetValueString(properties, "lxc.tty.max", ) <= 0) > return 0; if (virConfGetValueString(properties, "lxc.tty.max", ) <= 0) { virResetLastError(); /* Check for pre LXC 3.0 legacy key */ if (virConfGetValueString(properties, "lxc.tty", ) <= 0) return 0; } Although it could be argued that return 0 should also have a reset last error. > > if (virStrToLong_i(value, NULL, 10, ) < 0) { > @@ -724,7 +728,7 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr > value, void *data) > char type; > unsigned long start, target, count; > > -if (STRNEQ(name, "lxc.id_map") || !value->str) > +if (STRNEQ(name, "lxc.id_map") || STRNEQ(name, "lxc.idmap") || > !value->str) > return 0; > /* LXC 3.0 uses "lxc.idmap", while legacy used "lxc.id_map" */ if (STRNEQ(name, "lxc.idmap") || STRNEQ(name, "lxc.id_map") || !value->str) return 0; For this one, not only reverse the order, but also fix the error message: virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid %s: '%s'"), name, value->str); where name will be either "lxc.id_map" or "lxc.idmap" > if (sscanf(value->str, "%c %lu %lu %lu", , > @@ -1041,7 +1045,9 @@ lxcParseConfigString(const char *config, > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) > goto error; > > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || > +if ((virConfGetValueString(properties, "lxc.utsname", ) <= 0 && > + /* Legacy config keys were removed after release 3.0. */ > + virConfGetValueString(properties, "lxc.uts.name", ) <= 0) || > VIR_STRDUP(vmdef->name, value) < 0) > goto error; if (virConfGetValueString(properties, "lxc.uts.name", ) <= 0) { virResetLastError(); /* Check for pre LXC 3.0 legacy key */ if (virConfGetValueString(properties, "lxc.utsname", ) <= 0) goto error; } if (VIR_STRDUP(vmdef->name, value) < 0) goto error; > if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) > @@ -1051,7 +1057,9 @@ lxcParseConfigString(const char *config, > goto error; > > /* Look for fstab: we shouldn't have it */ > -if (virConfGetValue(properties, "lxc.mount")) { > +if (virConfGetValue(properties, "lxc.mount") || > +/* Legacy config keys were removed after release 3.0. */ > +virConfGetValue(properties, "lxc.mount.fstab")) { > virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > _("lxc.mount found, use lxc.mount.entry lines > instead")); > goto error; > This one's interesting because what does it mean if someone does use "lxc.mount.fstab"? Should they be using lxc.mount.entry instead? I assume this is what you want: /* LXC 3.0 uses "lxc.mount.fstab", while legacy used just "lxc.mount". * In either case, generate the error to use "lxc.mount.entry" instead */ if (virConfGetValue(properties, "lxc.mount.fstab") || virConfGetValue(properties, "lxc.mount")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("lxc.mount.fstab or
Re: [libvirt] [PATCH v2] lxc: Include support to lxc version 3.0 or higher.
On Monday, 5 November 2018 19:57:24 CET Julio Faracco wrote: > This patch introduce the new settings for LXC 3.0 or higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. Shouldn't it be better to check first for the new names of the configuration strings, using the pre-3.0 ones as fallback option? -- Pino Toscano signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: Include support to lxc version 3.0 or higher.
This patch introduce the new settings for LXC 3.0 or higher. The older versions keep the compatibility to deprecated settings for LXC, but after release 3.0, the compatibility was removed. This commit adds the support to the refactored settings. Signed-off-by: Julio Faracco --- src/lxc/lxc_native.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index cbdec723dd..8604cbaf46 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, int type = VIR_DOMAIN_FS_TYPE_MOUNT; VIR_AUTOFREE(char *) value = NULL; -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && +/* Legacy config keys were removed after release 3.0. */ +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) return -1; if (STRPREFIX(value, "/dev/")) @@ -684,7 +686,9 @@ lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties) virDomainChrDefPtr console; size_t i; -if (virConfGetValueString(properties, "lxc.tty", ) <= 0) +if (virConfGetValueString(properties, "lxc.tty", ) <= 0 && +/* Legacy config keys were removed after release 3.0. */ +virConfGetValueString(properties, "lxc.tty.max", ) <= 0) return 0; if (virStrToLong_i(value, NULL, 10, ) < 0) { @@ -724,7 +728,7 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr value, void *data) char type; unsigned long start, target, count; -if (STRNEQ(name, "lxc.id_map") || !value->str) +if (STRNEQ(name, "lxc.id_map") || STRNEQ(name, "lxc.idmap") || !value->str) return 0; if (sscanf(value->str, "%c %lu %lu %lu", , @@ -1041,7 +1045,9 @@ lxcParseConfigString(const char *config, if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) goto error; -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || +if ((virConfGetValueString(properties, "lxc.utsname", ) <= 0 && + /* Legacy config keys were removed after release 3.0. */ + virConfGetValueString(properties, "lxc.uts.name", ) <= 0) || VIR_STRDUP(vmdef->name, value) < 0) goto error; if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) @@ -1051,7 +1057,9 @@ lxcParseConfigString(const char *config, goto error; /* Look for fstab: we shouldn't have it */ -if (virConfGetValue(properties, "lxc.mount")) { +if (virConfGetValue(properties, "lxc.mount") || +/* Legacy config keys were removed after release 3.0. */ +virConfGetValue(properties, "lxc.mount.fstab")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("lxc.mount found, use lxc.mount.entry lines instead")); goto error; -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] lxc: Include support to lxc version 3.0 and higher.
On 10/01/2018 04:18 AM, Julio Faracco wrote: > This patch introduce the new settings for LXC 3.0 and higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_native.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > index cbdec723dd..c0f70ebb7d 100644 > --- a/src/lxc/lxc_native.c > +++ b/src/lxc/lxc_native.c > @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, > int type = VIR_DOMAIN_FS_TYPE_MOUNT; > VIR_AUTOFREE(char *) value = NULL; > > -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) > +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && > +/* Legacy config keys were removed after release 3.0. */ > +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) The intention looks good to me, but the formatting doesn't. Either: /* comment */ if (func() <= 0 && func() <= 0) return -1; or: if (func() <= 0) { /* comment */ if (func() <= 0) return -1; } Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] lxc: Include support to lxc version 3.0 and higher.
For further reference: https://lists.linuxcontainers.org/pipermail/lxc-devel/2018-April/017663.html Em dom, 30 de set de 2018 às 23:24, Julio Faracco escreveu: > > Hi guys, > > I put the RFC mark because I don't know if this is the best way to > enhance the support for LXC 3.0. Considering this patch, I can use > both o version without any issue, but I would like to see other points > of view. I'm not completely right about this approach. > > -- > Julio Cesar Faracco > > Em dom, 30 de set de 2018 às 23:19, Julio Faracco > escreveu: > > > > This patch introduce the new settings for LXC 3.0 and higher. The older > > versions keep the compatibility to deprecated settings for LXC, but > > after release 3.0, the compatibility was removed. This commit adds the > > support to the refactored settings. > > > > Signed-off-by: Julio Faracco > > --- > > src/lxc/lxc_native.c | 12 +--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > > index cbdec723dd..c0f70ebb7d 100644 > > --- a/src/lxc/lxc_native.c > > +++ b/src/lxc/lxc_native.c > > @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, > > int type = VIR_DOMAIN_FS_TYPE_MOUNT; > > VIR_AUTOFREE(char *) value = NULL; > > > > -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) > > +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && > > +/* Legacy config keys were removed after release 3.0. */ > > +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) > > return -1; > > > > if (STRPREFIX(value, "/dev/")) > > @@ -1041,7 +1043,9 @@ lxcParseConfigString(const char *config, > > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) > > goto error; > > > > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || > > +if ((virConfGetValueString(properties, "lxc.utsname", ) <= 0 && > > + /* Legacy config keys were removed after release 3.0. */ > > + virConfGetValueString(properties, "lxc.uts.name", ) <= 0) || > > VIR_STRDUP(vmdef->name, value) < 0) > > goto error; > > if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) > > @@ -1051,7 +1055,9 @@ lxcParseConfigString(const char *config, > > goto error; > > > > /* Look for fstab: we shouldn't have it */ > > -if (virConfGetValue(properties, "lxc.mount")) { > > +if (virConfGetValue(properties, "lxc.mount") || > > + /* Legacy config keys were removed after release 3.0. */ > > +virConfGetValue(properties, "lxc.mount.fstab")) { > > virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > > _("lxc.mount found, use lxc.mount.entry lines > > instead")); > > goto error; > > -- > > 2.17.1 > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] lxc: Include support to lxc version 3.0 and higher.
Hi guys, I put the RFC mark because I don't know if this is the best way to enhance the support for LXC 3.0. Considering this patch, I can use both o version without any issue, but I would like to see other points of view. I'm not completely right about this approach. -- Julio Cesar Faracco Em dom, 30 de set de 2018 às 23:19, Julio Faracco escreveu: > > This patch introduce the new settings for LXC 3.0 and higher. The older > versions keep the compatibility to deprecated settings for LXC, but > after release 3.0, the compatibility was removed. This commit adds the > support to the refactored settings. > > Signed-off-by: Julio Faracco > --- > src/lxc/lxc_native.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c > index cbdec723dd..c0f70ebb7d 100644 > --- a/src/lxc/lxc_native.c > +++ b/src/lxc/lxc_native.c > @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, > int type = VIR_DOMAIN_FS_TYPE_MOUNT; > VIR_AUTOFREE(char *) value = NULL; > > -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) > +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && > +/* Legacy config keys were removed after release 3.0. */ > +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) > return -1; > > if (STRPREFIX(value, "/dev/")) > @@ -1041,7 +1043,9 @@ lxcParseConfigString(const char *config, > if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) > goto error; > > -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || > +if ((virConfGetValueString(properties, "lxc.utsname", ) <= 0 && > + /* Legacy config keys were removed after release 3.0. */ > + virConfGetValueString(properties, "lxc.uts.name", ) <= 0) || > VIR_STRDUP(vmdef->name, value) < 0) > goto error; > if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) > @@ -1051,7 +1055,9 @@ lxcParseConfigString(const char *config, > goto error; > > /* Look for fstab: we shouldn't have it */ > -if (virConfGetValue(properties, "lxc.mount")) { > +if (virConfGetValue(properties, "lxc.mount") || > + /* Legacy config keys were removed after release 3.0. */ > +virConfGetValue(properties, "lxc.mount.fstab")) { > virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > _("lxc.mount found, use lxc.mount.entry lines > instead")); > goto error; > -- > 2.17.1 > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC] lxc: Include support to lxc version 3.0 and higher.
This patch introduce the new settings for LXC 3.0 and higher. The older versions keep the compatibility to deprecated settings for LXC, but after release 3.0, the compatibility was removed. This commit adds the support to the refactored settings. Signed-off-by: Julio Faracco --- src/lxc/lxc_native.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index cbdec723dd..c0f70ebb7d 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -200,7 +200,9 @@ lxcSetRootfs(virDomainDefPtr def, int type = VIR_DOMAIN_FS_TYPE_MOUNT; VIR_AUTOFREE(char *) value = NULL; -if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0) +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0 && +/* Legacy config keys were removed after release 3.0. */ +virConfGetValueString(properties, "lxc.rootfs.path", ) <= 0) return -1; if (STRPREFIX(value, "/dev/")) @@ -1041,7 +1043,9 @@ lxcParseConfigString(const char *config, if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0) goto error; -if (virConfGetValueString(properties, "lxc.utsname", ) <= 0 || +if ((virConfGetValueString(properties, "lxc.utsname", ) <= 0 && + /* Legacy config keys were removed after release 3.0. */ + virConfGetValueString(properties, "lxc.uts.name", ) <= 0) || VIR_STRDUP(vmdef->name, value) < 0) goto error; if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0)) @@ -1051,7 +1055,9 @@ lxcParseConfigString(const char *config, goto error; /* Look for fstab: we shouldn't have it */ -if (virConfGetValue(properties, "lxc.mount")) { +if (virConfGetValue(properties, "lxc.mount") || + /* Legacy config keys were removed after release 3.0. */ +virConfGetValue(properties, "lxc.mount.fstab")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("lxc.mount found, use lxc.mount.entry lines instead")); goto error; -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] lxc: Up back the veth interfaces by default
Hi again, Le jeudi 11 janvier 2018 à 18:08 +0100, Benjamin Cama a écrit : > Upping an interface without configuring it is not a “cardinal sin” but a > sensible way to achieve auto-configuration, e.g. with IPv6 SLAAC (RFC > 4862). If NetworkManager has troube with interfaces having only a > link-local address, this is a bug in NetworkManager, not in libvirt; it > should listen for router advertisements to decide if some interface has > global connectivity or not. To better understand my rant, a bit of context with the original patch proposal (whose message is also contained in the commit I pointed to): https://www.redhat.com/archives/libvir-list/2015-April/msg01062.html If you wonder why I react so late, this is because libvirt 3.0.0 landed in Debian Stretch (Jessie had 1.2.9!), and I just recently upgraded to it. > With network interfaces up by default, stateless containers can be > easily auto-configured through the network with SLAAC, without any > specific configuration from the host system. Of course, as a workaround I can use "", but I think it ought to be the default, hence my request. -- Benjamin Cama - Tél : 258 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH] lxc: Up back the veth interfaces by default
Upping an interface without configuring it is not a “cardinal sin” but a sensible way to achieve auto-configuration, e.g. with IPv6 SLAAC (RFC 4862). If NetworkManager has troube with interfaces having only a link-local address, this is a bug in NetworkManager, not in libvirt; it should listen for router advertisements to decide if some interface has global connectivity or not. With network interfaces up by default, stateless containers can be easily auto-configured through the network with SLAAC, without any specific configuration from the host system. This reverts commit c3cf3c43a0bb2e0e4909c32821e20f607635ec85. Signed-off-by: Benjamin Cama--- Hi, The patch that I propose to revert basically broke my workflow for light stateless containers, where they could be auto-configured on IPv6-only network through SLAAC. Of course, fully-fledged containers can bring up the interface themselves, but this behavior had previously been the default for quite some time, and is even indicated as default in src/conf/domain_conf.h ("Default link state (up)"). I cannot find any real justification for the patch I am reverting, and the bugzilla looks private so I can not comment on the NetworkManager behavior, which looks very buggy to me. Please tell me if you think this is wrong. Also, please Cc me, I am not subscribed. Regards. .mailmap| 1 + src/lxc/lxc_container.c | 7 +-- src/lxc/lxc_native.c| 10 ++ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/.mailmap b/.mailmap index 2f0fc901e..9dc3bff85 100644 --- a/.mailmap +++ b/.mailmap @@ -36,6 +36,7 @@ + diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 96fceaf1b..e546f0aaf 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -517,12 +517,7 @@ lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, if (virNetDevSetName(veths[i], newname) < 0) goto cleanup; -/* Only enable this device if there is a reason to do so (either - * at least one IP was specified, or link state was set to up in - * the config) - */ -if (netDef->guestIP.nips || -netDef->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) { +if (netDef->linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { VIR_DEBUG("Enabling %s", newname); if (virNetDevSetOnline(newname, true) < 0) goto cleanup; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index fdc03a57e..f77a2a910 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -349,10 +349,12 @@ lxcCreateNetDef(const char *type, if (VIR_ALLOC(net) < 0) goto error; -if (STREQ_NULLABLE(flag, "up")) -net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; -else -net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; +if (flag) { +if (STREQ(flag, "up")) +net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; +else +net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; +} if (VIR_STRDUP(net->ifname_guest, name) < 0) goto error; -- 2.11.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: monitor now holds a reference to the domain
On 08.12.2016 16:29, Cédric Bosdonnat wrote: > If the monitor doesn't hold a reference to the domain object > the object may be destroyed before the monitor actually stops. > --- > v2: Moved vm ref upper, removed vm unref in error case. > > src/lxc/lxc_monitor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c > index d828d528a..9cab6c203 100644 > --- a/src/lxc/lxc_monitor.c > +++ b/src/lxc/lxc_monitor.c > @@ -175,7 +175,7 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, > mon->program) < 0) > goto error; > > -mon->vm = vm; > +mon->vm = virObjectRef(vm); > memcpy(>cb, cb, sizeof(mon->cb)); > > virObjectRef(mon); > @@ -201,6 +201,7 @@ static void virLXCMonitorDispose(void *opaque) > if (mon->cb.destroy) > (mon->cb.destroy)(mon, mon->vm); > virObjectUnref(mon->program); > +virObjectUnref(mon->vm); > } > > > Yup, this one looks good. ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: monitor now holds a reference to the domain
If the monitor doesn't hold a reference to the domain object the object may be destroyed before the monitor actually stops. --- v2: Moved vm ref upper, removed vm unref in error case. src/lxc/lxc_monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c index d828d528a..9cab6c203 100644 --- a/src/lxc/lxc_monitor.c +++ b/src/lxc/lxc_monitor.c @@ -175,7 +175,7 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, mon->program) < 0) goto error; -mon->vm = vm; +mon->vm = virObjectRef(vm); memcpy(>cb, cb, sizeof(mon->cb)); virObjectRef(mon); @@ -201,6 +201,7 @@ static void virLXCMonitorDispose(void *opaque) if (mon->cb.destroy) (mon->cb.destroy)(mon, mon->vm); virObjectUnref(mon->program); +virObjectUnref(mon->vm); } -- 2.11.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ 3/5] lxc domain allow to set peer address
On Mon, Apr 04, 2016 at 09:00:04PM +, Vasiliy Tolstov wrote: > Signed-off-by: Vasiliy Tolstov> --- > src/lxc/lxc_container.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 348bbfbc01fc..a1deb0c00d4c 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -520,7 +520,7 @@ static int > lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, > > VIR_DEBUG("Adding IP address '%s/%u' to '%s'", >ipStr, ip->prefix, newname); > -if (virNetDevSetIPAddress(newname, >address, prefix) < 0) { > +if (virNetDevSetIPAddress(newname, >address, >peer, > prefix) < 0) { > virReportError(VIR_ERR_SYSTEM_ERROR, > _("Failed to set IP address '%s' on %s"), > ipStr, newname); ACK 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
[libvirt] [ 3/5] lxc domain allow to set peer address
Signed-off-by: Vasiliy Tolstov--- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 348bbfbc01fc..a1deb0c00d4c 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -520,7 +520,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, VIR_DEBUG("Adding IP address '%s/%u' to '%s'", ipStr, ip->prefix, newname); -if (virNetDevSetIPAddress(newname, >address, prefix) < 0) { +if (virNetDevSetIPAddress(newname, >address, >peer, prefix) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Failed to set IP address '%s' on %s"), ipStr, newname); -- 2.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
Just a one-time announcement - beside the git tree at github.com/hallyn/libresource there is also a mailing list now at https://lists.linuxcontainers.org/listinfo/libresource-devel I don't really intend to be a driving developer on it, but will happily review and discuss and help where I can. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
Quoting Daniel P. Berrange (berra...@redhat.com): > On Thu, Sep 24, 2015 at 03:53:24PM +, Serge Hallyn wrote: > > Quoting Daniel P. Berrange (berra...@redhat.com): > > > On Thu, Sep 24, 2015 at 02:41:49PM +, Serge Hallyn wrote: > > > > Quoting Fabio Kung (fabio.k...@gmail.com): > > > > > On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallyn > > > > >wrote: > > > > > > > > > > > > Ok, so I could create a project on github, but that doesn't come > > > > > > with > > > > > > a m-l. Last I used it, sf was problematic. Any other suggestions > > > > > > for > > > > > > where to host a mailing list? Might the github issue tracker > > > > > > suffice? > > > > > > We could (as worked quite well for lxd) have a specs/ directory in a > > > > > > libresource source tree, and use issues and pull reuqests to guide > > > > > > the > > > > > > api specifications under that directory. Just a thought. > > > > > > > > > > This all sgtm. A mailing list for design discussions + github issue > > > > > tracker seems to be working well for many open source projects I've > > > > > been tracking lately. Most of them are using Google Groups for their > > > > > mailing lists. > > > > > > > > Well for starters I created https://github.com/hallyn/libresource . We > > > > should create a real project for it but it's a start. (I'll create an > > > > organization if this starts to move) > > > > > > > > Actually I suppose the first step would be deciding on a license. > > > > Normally > > > > I default to gplv2, but for this that may not be appropriate. Apache > > > > license? Can be settled in an issue or pull request for a License file, > > > > I think. > > > > > > My personal preference is always LGPLv2+ for libraries, since it gives > > > ability to use from non-open source apps, but is still copyleft. I know > > > corporates tend to prefer non-copyleft licenses like Apache these days, > > > but that is generally for ulterior motives like being able to do dual > > > open/closed products. > > > > I think one of the most important consumers would be procps, and this > > wouldn't be an issue for them. Now one of the reasons we want this is > > so that software like databases and big java apps can check their > > real available resources to scale - would this be an issue for them, > > or do we think they would just link to or execute commands from > > procps? > > I guess where it could become an issue is if $BIGVENDOR wants to bundle > a copy of the library statically with their app. Some companies are > (irrationally) paranoid about shipping anything copyleft themselves, > so Apache could suit that. Its a tradeoff, as it obviously lets them > embrace & extend rather than forcing them to share improvements they > make. Agreed. https://github.com/hallyn/libresource/pull/2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Thu, Sep 24, 2015 at 02:41:49PM +, Serge Hallyn wrote: > Quoting Fabio Kung (fabio.k...@gmail.com): > > On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallyn> > wrote: > > > > > > Ok, so I could create a project on github, but that doesn't come with > > > a m-l. Last I used it, sf was problematic. Any other suggestions for > > > where to host a mailing list? Might the github issue tracker suffice? > > > We could (as worked quite well for lxd) have a specs/ directory in a > > > libresource source tree, and use issues and pull reuqests to guide the > > > api specifications under that directory. Just a thought. > > > > This all sgtm. A mailing list for design discussions + github issue > > tracker seems to be working well for many open source projects I've > > been tracking lately. Most of them are using Google Groups for their > > mailing lists. > > Well for starters I created https://github.com/hallyn/libresource . We > should create a real project for it but it's a start. (I'll create an > organization if this starts to move) > > Actually I suppose the first step would be deciding on a license. Normally > I default to gplv2, but for this that may not be appropriate. Apache > license? Can be settled in an issue or pull request for a License file, > I think. My personal preference is always LGPLv2+ for libraries, since it gives ability to use from non-open source apps, but is still copyleft. I know corporates tend to prefer non-copyleft licenses like Apache these days, but that is generally for ulterior motives like being able to do dual open/closed products. 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 v2] lxc: fuse mount for /proc/cpuinfo
Quoting Fabio Kung (fabio.k...@gmail.com): > On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallyn> wrote: > > > > Ok, so I could create a project on github, but that doesn't come with > > a m-l. Last I used it, sf was problematic. Any other suggestions for > > where to host a mailing list? Might the github issue tracker suffice? > > We could (as worked quite well for lxd) have a specs/ directory in a > > libresource source tree, and use issues and pull reuqests to guide the > > api specifications under that directory. Just a thought. > > This all sgtm. A mailing list for design discussions + github issue > tracker seems to be working well for many open source projects I've > been tracking lately. Most of them are using Google Groups for their > mailing lists. Well for starters I created https://github.com/hallyn/libresource . We should create a real project for it but it's a start. (I'll create an organization if this starts to move) Actually I suppose the first step would be deciding on a license. Normally I default to gplv2, but for this that may not be appropriate. Apache license? Can be settled in an issue or pull request for a License file, I think. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
Quoting Daniel P. Berrange (berra...@redhat.com): > On Thu, Sep 24, 2015 at 02:41:49PM +, Serge Hallyn wrote: > > Quoting Fabio Kung (fabio.k...@gmail.com): > > > On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallyn> > > wrote: > > > > > > > > Ok, so I could create a project on github, but that doesn't come with > > > > a m-l. Last I used it, sf was problematic. Any other suggestions for > > > > where to host a mailing list? Might the github issue tracker suffice? > > > > We could (as worked quite well for lxd) have a specs/ directory in a > > > > libresource source tree, and use issues and pull reuqests to guide the > > > > api specifications under that directory. Just a thought. > > > > > > This all sgtm. A mailing list for design discussions + github issue > > > tracker seems to be working well for many open source projects I've > > > been tracking lately. Most of them are using Google Groups for their > > > mailing lists. > > > > Well for starters I created https://github.com/hallyn/libresource . We > > should create a real project for it but it's a start. (I'll create an > > organization if this starts to move) > > > > Actually I suppose the first step would be deciding on a license. Normally > > I default to gplv2, but for this that may not be appropriate. Apache > > license? Can be settled in an issue or pull request for a License file, > > I think. > > My personal preference is always LGPLv2+ for libraries, since it gives > ability to use from non-open source apps, but is still copyleft. I know > corporates tend to prefer non-copyleft licenses like Apache these days, > but that is generally for ulterior motives like being able to do dual > open/closed products. > > Regards, > Daniel I think one of the most important consumers would be procps, and this wouldn't be an issue for them. Now one of the reasons we want this is so that software like databases and big java apps can check their real available resources to scale - would this be an issue for them, or do we think they would just link to or execute commands from procps? -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Thu, Sep 24, 2015 at 03:53:24PM +, Serge Hallyn wrote: > Quoting Daniel P. Berrange (berra...@redhat.com): > > On Thu, Sep 24, 2015 at 02:41:49PM +, Serge Hallyn wrote: > > > Quoting Fabio Kung (fabio.k...@gmail.com): > > > > On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallyn > > > >wrote: > > > > > > > > > > Ok, so I could create a project on github, but that doesn't come with > > > > > a m-l. Last I used it, sf was problematic. Any other suggestions for > > > > > where to host a mailing list? Might the github issue tracker suffice? > > > > > We could (as worked quite well for lxd) have a specs/ directory in a > > > > > libresource source tree, and use issues and pull reuqests to guide the > > > > > api specifications under that directory. Just a thought. > > > > > > > > This all sgtm. A mailing list for design discussions + github issue > > > > tracker seems to be working well for many open source projects I've > > > > been tracking lately. Most of them are using Google Groups for their > > > > mailing lists. > > > > > > Well for starters I created https://github.com/hallyn/libresource . We > > > should create a real project for it but it's a start. (I'll create an > > > organization if this starts to move) > > > > > > Actually I suppose the first step would be deciding on a license. > > > Normally > > > I default to gplv2, but for this that may not be appropriate. Apache > > > license? Can be settled in an issue or pull request for a License file, > > > I think. > > > > My personal preference is always LGPLv2+ for libraries, since it gives > > ability to use from non-open source apps, but is still copyleft. I know > > corporates tend to prefer non-copyleft licenses like Apache these days, > > but that is generally for ulterior motives like being able to do dual > > open/closed products. > > I think one of the most important consumers would be procps, and this > wouldn't be an issue for them. Now one of the reasons we want this is > so that software like databases and big java apps can check their > real available resources to scale - would this be an issue for them, > or do we think they would just link to or execute commands from > procps? I guess where it could become an issue is if $BIGVENDOR wants to bundle a copy of the library statically with their app. Some companies are (irrationally) paranoid about shipping anything copyleft themselves, so Apache could suit that. Its a tradeoff, as it obviously lets them embrace & extend rather than forcing them to share improvements they make. 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 v2] lxc: fuse mount for /proc/cpuinfo
On Wed, 2015-09-16 at 19:29 +, Serge Hallyn wrote: > Quoting Daniel P. Berrange (berra...@redhat.com): > > On Wed, Sep 16, 2015 at 03:15:52PM +, Serge Hallyn wrote: > > > Quoting Fabio Kung (fabio.k...@gmail.com): > > > > On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn <serge.hal...@ubuntu.com> > > > > wrote: > > > > > > > > > > Ah, my memory was failing me, so took a bit of searching, but > > > > > > > > > > http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ > > > > > > > > > > I can't find anything called 'libmymem', and in 2014 he said > > > > > > > > > > https://github.com/docker/docker/issues/8427#issuecomment-58255159 > > > > > > > > > > so maybe this never went anywhere. > > > > > > > > Correct, unfortunately. > > > > > > > > > > > > > For the same reasons you cited above, and because everyeone is rolling > > > > > their own at fuse level, I still think that a libresource and patches > > > > > to proc tools to use them, is the right way to go. We have no > > > > > shortage > > > > > of sample code for the functions doing the actual work, between > > > > > libvirt, > > > > > lxc, docker, etc :) > > > > > > > > > > Should we just go ahead and start a libresource github project? > > > > > > > > +1, if there's momentum on this I believe I will be able to contribute > > > > some cycles. Maybe now is the right time? > > > > > > Might be. Maybe the thing to do is start a project and mailing list > > > (any objections to github? Do we create a new project for this?), and > > > see if more than 3 people join :) Announce on containers@ and cgroup@ > > > mailing lists, and start discussing what a reasonable API would look > > > like. > > > > FWIW, I would support any such effort, but I'm unlikely to have free > > resources to do anything more than watch its mailing list. > > NP - if you can correct our course if we're heading someplace bad for > libvirt that'll be great. Though I suspect lxc/lxd and libvirt will > mostly agree. I can possibly help the coding... though I'm not too versed in the low-level things (yet), don't count on me as one of the main hackers ;) > Ok, so I could create a project on github, but that doesn't come with > a m-l. Last I used it, sf was problematic. Any other suggestions for > where to host a mailing list? Might the github issue tracker suffice? > We could (as worked quite well for lxd) have a specs/ directory in a > libresource source tree, and use issues and pull reuqests to guide the > api specifications under that directory. Just a thought. It could be OK to start with the github issue tracker and we'll see if a mailing list is really needed. I'm using SF.net for other projects and I feel it's always a pain to use. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Wed, Sep 16, 2015 at 12:29 PM, Serge Hallynwrote: > > Ok, so I could create a project on github, but that doesn't come with > a m-l. Last I used it, sf was problematic. Any other suggestions for > where to host a mailing list? Might the github issue tracker suffice? > We could (as worked quite well for lxd) have a specs/ directory in a > libresource source tree, and use issues and pull reuqests to guide the > api specifications under that directory. Just a thought. This all sgtm. A mailing list for design discussions + github issue tracker seems to be working well for many open source projects I've been tracking lately. Most of them are using Google Groups for their mailing lists. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
Quoting Fabio Kung (fabio.k...@gmail.com): > On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn <serge.hal...@ubuntu.com> wrote: > > > > Ah, my memory was failing me, so took a bit of searching, but > > > > http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ > > > > I can't find anything called 'libmymem', and in 2014 he said > > > > https://github.com/docker/docker/issues/8427#issuecomment-58255159 > > > > so maybe this never went anywhere. > > Correct, unfortunately. > > > > For the same reasons you cited above, and because everyeone is rolling > > their own at fuse level, I still think that a libresource and patches > > to proc tools to use them, is the right way to go. We have no shortage > > of sample code for the functions doing the actual work, between libvirt, > > lxc, docker, etc :) > > > > Should we just go ahead and start a libresource github project? > > +1, if there's momentum on this I believe I will be able to contribute > some cycles. Maybe now is the right time? Might be. Maybe the thing to do is start a project and mailing list (any objections to github? Do we create a new project for this?), and see if more than 3 people join :) Announce on containers@ and cgroup@ mailing lists, and start discussing what a reasonable API would look like. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Wed, Sep 16, 2015 at 03:15:52PM +, Serge Hallyn wrote: > Quoting Fabio Kung (fabio.k...@gmail.com): > > On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn <serge.hal...@ubuntu.com> > > wrote: > > > > > > Ah, my memory was failing me, so took a bit of searching, but > > > > > > http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ > > > > > > I can't find anything called 'libmymem', and in 2014 he said > > > > > > https://github.com/docker/docker/issues/8427#issuecomment-58255159 > > > > > > so maybe this never went anywhere. > > > > Correct, unfortunately. > > > > > > > For the same reasons you cited above, and because everyeone is rolling > > > their own at fuse level, I still think that a libresource and patches > > > to proc tools to use them, is the right way to go. We have no shortage > > > of sample code for the functions doing the actual work, between libvirt, > > > lxc, docker, etc :) > > > > > > Should we just go ahead and start a libresource github project? > > > > +1, if there's momentum on this I believe I will be able to contribute > > some cycles. Maybe now is the right time? > > Might be. Maybe the thing to do is start a project and mailing list > (any objections to github? Do we create a new project for this?), and > see if more than 3 people join :) Announce on containers@ and cgroup@ > mailing lists, and start discussing what a reasonable API would look > like. FWIW, I would support any such effort, but I'm unlikely to have free resources to do anything more than watch its mailing list. 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 v2] lxc: fuse mount for /proc/cpuinfo
Quoting Daniel P. Berrange (berra...@redhat.com): > On Wed, Sep 16, 2015 at 03:15:52PM +, Serge Hallyn wrote: > > Quoting Fabio Kung (fabio.k...@gmail.com): > > > On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn <serge.hal...@ubuntu.com> > > > wrote: > > > > > > > > Ah, my memory was failing me, so took a bit of searching, but > > > > > > > > http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ > > > > > > > > I can't find anything called 'libmymem', and in 2014 he said > > > > > > > > https://github.com/docker/docker/issues/8427#issuecomment-58255159 > > > > > > > > so maybe this never went anywhere. > > > > > > Correct, unfortunately. > > > > > > > > > > For the same reasons you cited above, and because everyeone is rolling > > > > their own at fuse level, I still think that a libresource and patches > > > > to proc tools to use them, is the right way to go. We have no shortage > > > > of sample code for the functions doing the actual work, between libvirt, > > > > lxc, docker, etc :) > > > > > > > > Should we just go ahead and start a libresource github project? > > > > > > +1, if there's momentum on this I believe I will be able to contribute > > > some cycles. Maybe now is the right time? > > > > Might be. Maybe the thing to do is start a project and mailing list > > (any objections to github? Do we create a new project for this?), and > > see if more than 3 people join :) Announce on containers@ and cgroup@ > > mailing lists, and start discussing what a reasonable API would look > > like. > > FWIW, I would support any such effort, but I'm unlikely to have free > resources to do anything more than watch its mailing list. NP - if you can correct our course if we're heading someplace bad for libvirt that'll be great. Though I suspect lxc/lxd and libvirt will mostly agree. Ok, so I could create a project on github, but that doesn't come with a m-l. Last I used it, sf was problematic. Any other suggestions for where to host a mailing list? Might the github issue tracker suffice? We could (as worked quite well for lxd) have a specs/ directory in a libresource source tree, and use issues and pull reuqests to guide the api specifications under that directory. Just a thought. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Mon, Sep 7, 2015 at 8:55 AM, Serge Hallyn <serge.hal...@ubuntu.com> wrote: > > Ah, my memory was failing me, so took a bit of searching, but > > http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ > > I can't find anything called 'libmymem', and in 2014 he said > > https://github.com/docker/docker/issues/8427#issuecomment-58255159 > > so maybe this never went anywhere. Correct, unfortunately. > For the same reasons you cited above, and because everyeone is rolling > their own at fuse level, I still think that a libresource and patches > to proc tools to use them, is the right way to go. We have no shortage > of sample code for the functions doing the actual work, between libvirt, > lxc, docker, etc :) > > Should we just go ahead and start a libresource github project? +1, if there's momentum on this I believe I will be able to contribute some cycles. Maybe now is the right time? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote: > We already have a fuse mount to reflect the cgroup memory restrictions > in the container. This commit adds the same for the number of available > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the > container's cpuinfo. So this (re-)raises some interesting / difficult questions that I'm not sure we have a good answer to. The main concern is that actually this is not really a problem specific to containers, rather it is related to cgroup resource confinement. ie the cgroup has confined a process(es) to a set of CPUs are the process is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being increasingly widely used in Linux, particularly since systemd, so pretty much any process has to expect that it can be confined to a subset of CPUs. IOW, any application using /proc/cpuinfo to determine "available" resource is already broken, even when run on bare metal. The same also applies to the use of /proc/meminfo, which we previously faked via fuse. So the question is whether we should invest time trying to fake the /proc/cpuinfo in containers, when any apps we'd be fixing are already broken in bare metal. Apps might have avoided /proc/cpuinfo and instead be trying /sys/devices/system/cpu/ which your patch isn't trying to fake. This is just as broken, because sysfs doesn't reflect cgroup confinement either. I think what is ultimately needed for applications is some kind of libresource.so library that they can use to query what resources are available in their compute environment, which can intelligently query cgroups directly, and ignore the legacy /proc & /sys interfaces for counting memory / cpu availability. I don't think that's something that libvirt should solve - if anything it could be systemd, or a standalone project. So I'm increasingly convinced that LXC should not try to fake out any /proc & /sys file content, and instead document the limitations. I'm also thinking that we should kill off our existing meminfo fake fuse at some point. The more minor concern I have is around the implementation. AFAIR, the /proc/cpuinfo file contents is not standardized across architectures, so I'm concerned whether your parsing code is robust on non-x86 arches. 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 v2] lxc: fuse mount for /proc/cpuinfo
Quoting Daniel P. Berrange (berra...@redhat.com): > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote: > > We already have a fuse mount to reflect the cgroup memory restrictions > > in the container. This commit adds the same for the number of available > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the > > container's cpuinfo. > > So this (re-)raises some interesting / difficult questions that I'm > not sure we have a good answer to. > > The main concern is that actually this is not really a problem specific > to containers, rather it is related to cgroup resource confinement. > ie the cgroup has confined a process(es) to a set of CPUs are the process > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being > increasingly widely used in Linux, particularly since systemd, so pretty > much any process has to expect that it can be confined to a subset of > CPUs. > > IOW, any application using /proc/cpuinfo to determine "available" > resource is already broken, even when run on bare metal. The same also > applies to the use of /proc/meminfo, which we previously faked via > fuse. > > So the question is whether we should invest time trying to fake the > /proc/cpuinfo in containers, when any apps we'd be fixing are already > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead > be trying /sys/devices/system/cpu/ which your patch isn't trying to > fake. This is just as broken, because sysfs doesn't reflect cgroup > confinement either. > > I think what is ultimately needed for applications is some kind of > libresource.so library that they can use to query what resources Does anyone remember who it was that announced an effort to this end a year or two ago, or know what the status of it is? > are available in their compute environment, which can intelligently > query cgroups directly, and ignore the legacy /proc & /sys interfaces > for counting memory / cpu availability. I don't think that's something > that libvirt should solve - if anything it could be systemd, or a > standalone project. > > So I'm increasingly convinced that LXC should not try to fake out > any /proc & /sys file content, and instead document the limitations. > I'm also thinking that we should kill off our existing meminfo fake > fuse at some point. > > The more minor concern I have is around the implementation. AFAIR, the > /proc/cpuinfo file contents is not standardized across architectures, > so I'm concerned whether your parsing code is robust on non-x86 arches. > > 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
Quoting Daniel P. Berrange (berra...@redhat.com): > On Mon, Sep 07, 2015 at 03:39:13PM +, Serge Hallyn wrote: > > Quoting Daniel P. Berrange (berra...@redhat.com): > > > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote: > > > > We already have a fuse mount to reflect the cgroup memory restrictions > > > > in the container. This commit adds the same for the number of available > > > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the > > > > container's cpuinfo. > > > > > > So this (re-)raises some interesting / difficult questions that I'm > > > not sure we have a good answer to. > > > > > > The main concern is that actually this is not really a problem specific > > > to containers, rather it is related to cgroup resource confinement. > > > ie the cgroup has confined a process(es) to a set of CPUs are the process > > > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being > > > increasingly widely used in Linux, particularly since systemd, so pretty > > > much any process has to expect that it can be confined to a subset of > > > CPUs. > > > > > > IOW, any application using /proc/cpuinfo to determine "available" > > > resource is already broken, even when run on bare metal. The same also > > > applies to the use of /proc/meminfo, which we previously faked via > > > fuse. > > > > > > So the question is whether we should invest time trying to fake the > > > /proc/cpuinfo in containers, when any apps we'd be fixing are already > > > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead > > > be trying /sys/devices/system/cpu/ which your patch isn't trying to > > > fake. This is just as broken, because sysfs doesn't reflect cgroup > > > confinement either. > > > > > > I think what is ultimately needed for applications is some kind of > > > libresource.so library that they can use to query what resources > > > > Does anyone remember who it was that announced an effort to this > > end a year or two ago, or know what the status of it is? > > I don't recall seeing any formal announcement about this, but I have > had this exact same discussion with Red Hat folks involved with > Docker and similar higher level container mgmt tools, so perhaps > someone involved in those efforts is working on it ? Ah, my memory was failing me, so took a bit of searching, but http://fabiokung.com/2014/03/13/memory-inside-linux-containers/ I can't find anything called 'libmymem', and in 2014 he said https://github.com/docker/docker/issues/8427#issuecomment-58255159 so maybe this never went anywhere. For the same reasons you cited above, and because everyeone is rolling their own at fuse level, I still think that a libresource and patches to proc tools to use them, is the right way to go. We have no shortage of sample code for the functions doing the actual work, between libvirt, lxc, docker, etc :) Should we just go ahead and start a libresource github project? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Mon, 2015-09-07 at 15:21 +0200, Cedric Bosdonnat wrote: > > The more minor concern I have is around the implementation. AFAIR, > > the > > /proc/cpuinfo file contents is not standardized across > > architectures, > > so I'm concerned whether your parsing code is robust on non-x86 > > arches. > > Hum... I didn't even know that file would change with arch'es. Take a look at linuxNodeInfoCPUPopulate() in src/nodeinfo.c for inspiration. Sharing the parsing code would also be nice. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
On Mon, Sep 07, 2015 at 03:39:13PM +, Serge Hallyn wrote: > Quoting Daniel P. Berrange (berra...@redhat.com): > > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote: > > > We already have a fuse mount to reflect the cgroup memory restrictions > > > in the container. This commit adds the same for the number of available > > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the > > > container's cpuinfo. > > > > So this (re-)raises some interesting / difficult questions that I'm > > not sure we have a good answer to. > > > > The main concern is that actually this is not really a problem specific > > to containers, rather it is related to cgroup resource confinement. > > ie the cgroup has confined a process(es) to a set of CPUs are the process > > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being > > increasingly widely used in Linux, particularly since systemd, so pretty > > much any process has to expect that it can be confined to a subset of > > CPUs. > > > > IOW, any application using /proc/cpuinfo to determine "available" > > resource is already broken, even when run on bare metal. The same also > > applies to the use of /proc/meminfo, which we previously faked via > > fuse. > > > > So the question is whether we should invest time trying to fake the > > /proc/cpuinfo in containers, when any apps we'd be fixing are already > > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead > > be trying /sys/devices/system/cpu/ which your patch isn't trying to > > fake. This is just as broken, because sysfs doesn't reflect cgroup > > confinement either. > > > > I think what is ultimately needed for applications is some kind of > > libresource.so library that they can use to query what resources > > Does anyone remember who it was that announced an effort to this > end a year or two ago, or know what the status of it is? I don't recall seeing any formal announcement about this, but I have had this exact same discussion with Red Hat folks involved with Docker and similar higher level container mgmt tools, so perhaps someone involved in those efforts is working on it ? 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 v2] lxc: fuse mount for /proc/cpuinfo
On Mon, 2015-09-07 at 13:23 +0100, Daniel P. Berrange wrote: > On Thu, Sep 03, 2015 at 11:51:16AM +0200, Cédric Bosdonnat wrote: > > We already have a fuse mount to reflect the cgroup memory restrictions > > in the container. This commit adds the same for the number of available > > CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the > > container's cpuinfo. > > So this (re-)raises some interesting / difficult questions that I'm > not sure we have a good answer to. > > The main concern is that actually this is not really a problem specific > to containers, rather it is related to cgroup resource confinement. > ie the cgroup has confined a process(es) to a set of CPUs are the process > is using /proc/cpuinfo to count CPUs and so is wrong. Cgroups are being > increasingly widely used in Linux, particularly since systemd, so pretty > much any process has to expect that it can be confined to a subset of > CPUs. I agree. > IOW, any application using /proc/cpuinfo to determine "available" > resource is already broken, even when run on bare metal. The same also > applies to the use of /proc/meminfo, which we previously faked via > fuse. > > So the question is whether we should invest time trying to fake the > /proc/cpuinfo in containers, when any apps we'd be fixing are already > broken in bare metal. Apps might have avoided /proc/cpuinfo and instead > be trying /sys/devices/system/cpu/ which your patch isn't trying to > fake. This is just as broken, because sysfs doesn't reflect cgroup > confinement either. I agree /sys/devices/system/cpu should be patched too... but it contains much more subtle things to handle. At least I don't have a good enough knowledge of that FS to fake it properly. > I think what is ultimately needed for applications is some kind of > libresource.so library that they can use to query what resources > are available in their compute environment, which can intelligently > query cgroups directly, and ignore the legacy /proc & /sys interfaces > for counting memory / cpu availability. I don't think that's something > that libvirt should solve - if anything it could be systemd, or a > standalone project. Ok, then not something that would be available in a reasonable time frame unless we start it. Do you know if someone in another project is caring about that problem? > So I'm increasingly convinced that LXC should not try to fake out > any /proc & /sys file content, and instead document the limitations. > I'm also thinking that we should kill off our existing meminfo fake > fuse at some point. OK. > The more minor concern I have is around the implementation. AFAIR, the > /proc/cpuinfo file contents is not standardized across architectures, > so I'm concerned whether your parsing code is robust on non-x86 arches. Hum... I didn't even know that file would change with arch'es. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: fuse mount for /proc/cpuinfo
We already have a fuse mount to reflect the cgroup memory restrictions in the container. This commit adds the same for the number of available CPUs. Only the CPUs listed by virProcessGetAffinity are shown in the container's cpuinfo. --- src/lxc/lxc_container.c | 42 --- src/lxc/lxc_fuse.c | 106 +++- 2 files changed, 133 insertions(+), 15 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a433552..7ae13a8 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1055,24 +1055,38 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def, const char *stateDir) { int ret; -char *meminfo_path = NULL; +char *src_path = NULL; +char *dst_path = NULL; +const char *paths[] = {"meminfo", "cpuinfo"}; +size_t i; -VIR_DEBUG("Mount /proc/meminfo stateDir=%s", stateDir); +for (i = 0; i < 2; i++) { +VIR_DEBUG("Mount /proc/%s stateDir=%s", paths[i], stateDir); + +if ((ret = virAsprintf(_path, + "/.oldroot/%s/%s.fuse/%s", + stateDir, + def->name, + paths[i])) < 0) +return ret; + +if ((ret = virAsprintf(_path, + "/proc/%s", + paths[i])) < 0) { +VIR_FREE(src_path); +return ret; +} -if ((ret = virAsprintf(_path, - "/.oldroot/%s/%s.fuse/meminfo", - stateDir, - def->name)) < 0) -return ret; +if ((ret = mount(src_path, dst_path, + NULL, MS_BIND, NULL)) < 0) { +virReportSystemError(errno, + _("Failed to mount %s on %s"), + src_path, dst_path); +} -if ((ret = mount(meminfo_path, "/proc/meminfo", - NULL, MS_BIND, NULL)) < 0) { -virReportSystemError(errno, - _("Failed to mount %s on /proc/meminfo"), - meminfo_path); +VIR_FREE(src_path); +VIR_FREE(dst_path); } - -VIR_FREE(meminfo_path); return ret; } #else diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 34a69cc..0d60434 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -42,6 +42,58 @@ #if WITH_FUSE static const char *fuse_meminfo_path = "/meminfo"; +static const char *fuse_cpuinfo_path = "/cpuinfo"; + +static virBufferPtr lxcProcComputeCpuinfo(void) { +FILE *fd = NULL; +char *line = NULL; +size_t n; +bool writeProc = false; +virBuffer buffer = VIR_BUFFER_INITIALIZER; +virBufferPtr new_cpuinfo = +pid_t pid; +virBitmapPtr cpuAffinity = NULL; + +fd = fopen("/proc/cpuinfo", "r"); +if (fd == NULL) { +virReportSystemError(errno, "%s", _("Cannot open /proc/cpuinfo")); +goto error; +} + +pid = getpid(); +if (!(cpuAffinity = virProcessGetAffinity(pid))) +goto error; + +while (getline(, , fd) > 0) { +if (STRPREFIX(line, "processor\t:")) { +unsigned long cpuid = 0; +char *suffix = NULL; +if (virStrToLong_ul(line + 12, , 10, ) < 0) +goto error; + +if (virBitmapGetBit(cpuAffinity, cpuid, ) < 0) +goto error; +} + +if (writeProc) { +virBufferAdd(new_cpuinfo, line, -1); + +if (virBufferCheckError(new_cpuinfo) < 0) +goto error; +} +} + + cleanup: +VIR_FREE(line); +VIR_FORCE_FCLOSE(fd); +virBitmapFree(cpuAffinity); +return new_cpuinfo; + + error: +virBufferFreeAndReset(new_cpuinfo); +new_cpuinfo = NULL; +goto cleanup; +} static int lxcProcGetattr(const char *path, struct stat *stbuf) { @@ -50,6 +102,7 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) struct stat sb; struct fuse_context *context = fuse_get_context(); virDomainDefPtr def = (virDomainDefPtr)context->private_data; +virBufferPtr cpuinfo = NULL; memset(stbuf, 0, sizeof(struct stat)); if (virAsprintf(, "/proc/%s", path) < 0) @@ -76,12 +129,36 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) stbuf->st_atime = sb.st_atime; stbuf->st_ctime = sb.st_ctime; stbuf->st_mtime = sb.st_mtime; +} else if (STREQ(path, fuse_cpuinfo_path)) { +if (!(cpuinfo = lxcProcComputeCpuinfo())) { +res = -EIO; +goto cleanup; +} + +if (stat(mempath, ) < 0) { +res = -errno; +goto cleanup; +} + +stbuf->st_uid = def->idmap.uidmap ? def->idmap.uidmap[0].target : 0; +stbuf->st_gid = def->idmap.gidmap ? def->idmap.gidmap[0].target : 0; +
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
On Thu, Aug 27, 2015 at 04:47:24PM -0600, Eric Blake wrote: On 08/27/2015 04:38 PM, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Yes, that sounds like the best way to handle it. Agreed 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] Revert LXC: show used memory as 0 when domain is not active
Eric Blake wrote: On 08/27/2015 04:38 PM, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Yes, that sounds like the best way to handle it. Ok, pushed to master and v1.2.7-maint through v1.2.18-maint. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
Daniel P. Berrange wrote: On Tue, Aug 11, 2015 at 10:25:19AM +0200, Peter Krempa wrote: On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote: On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
On 08/27/2015 04:38 PM, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Yes, that sounds like the best way to handle it. -- 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 v3] lxc: Inherit namespace feature
On 08/14/2015 08:09 AM, Daniel P. Berrange wrote: From: Imran Khan ik.n...@gmail.com This patch adds feature for lxc containers to inherit namespaces. This is very similar to what lxc-tools or docker provides. Look for man lxc-start and you will find that you can pass command args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker networking option in which you can give --net=container:NAME_or_ID as an option for sharing +namespace. From this patch you can add extra libvirt option to share namespace in following way. lxc:namespace lxc:sharenet type='netns' value='red'/ lxc:shareipc type='pid' value='12345'/ lxc:shareuts type='name' value='container1'/ /lxc:namespace The netns option is specific to sharenet. It can be used to inherit from existing network namespace. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/drvlxc.html.in | 21 ++ docs/schemas/domaincommon.rng | 42 po/POTFILES.in| 1 + src/Makefile.am | 6 +- src/lxc/lxc_conf.c| 2 +- src/lxc/lxc_container.c | 71 ++-- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 45 - src/lxc/lxc_domain.c | 149 ++ src/lxc/lxc_domain.h | 26 src/lxc/lxc_process.c | 149 ++ tests/lxcxml2xmltest.c| 1 + 12 files changed, 506 insertions(+), 9 deletions(-) ... Coverity found a resource leak... @@ -2342,6 +2378,7 @@ int lxcContainerStart(virDomainDefPtr def, int *passFDs, int control, int handshakefd, + int *nsInheritFDs, size_t nttyPaths, char **ttyPaths) { @@ -2359,7 +2396,8 @@ int lxcContainerStart(virDomainDefPtr def, .monitor = control, .nttyPaths = nttyPaths, .ttyPaths = ttyPaths, -.handshakefd = handshakefd +.handshakefd = handshakefd, +.nsInheritFDs = nsInheritFDs, }; /* allocate a stack for the container */ @@ -2368,7 +2406,7 @@ int lxcContainerStart(virDomainDefPtr def, stacktop = stack + stacksize; -cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; +cflags = CLONE_NEWPID|CLONE_NEWNS|SIGCHLD; if (userns_required(def)) { if (userns_supported()) { @@ -2381,10 +2419,31 @@ int lxcContainerStart(virDomainDefPtr def, return -1; } } +if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHARENET] == -1) { +if (lxcNeedNetworkNamespace(def)) { +VIR_DEBUG(Enable network namespaces); +cflags |= CLONE_NEWNET; +} +} else { +if (lxcNeedNetworkNamespace(def)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Config askes for inherit net namespace + as well as private network interfaces)); +return -1; This leaks 'stack'... Sending a patch shortly. John +} +VIR_DEBUG(Inheriting a net namespace); +} -if (lxcNeedNetworkNamespace(def)) { -VIR_DEBUG(Enable network namespaces); -cflags |= CLONE_NEWNET; +if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHAREIPC] == -1) { +cflags |= CLONE_NEWIPC; +} else { +VIR_DEBUG(Inheriting an IPC namespace); +} + +if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHAREUTS] == -1) { +cflags |= CLONE_NEWUTS; +} else { +VIR_DEBUG(Inheriting a UTS namespace); } VIR_DEBUG(Cloning container init process); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] lxc: Inherit namespace feature
On Thu, Aug 20, 2015 at 07:16:17PM +0530, ik.nitk wrote: This patch adds feature for lxc containers to inherit namespaces. This is very similar to what lxc-tools or docker provides. Look for man lxc-start and you will find that you can pass command args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker networking option in which you can give --net=container:NAME_or_ID as an option for sharing +namespace. From this patch you can add extra libvirt option to share namespace in following way. lxc:namespace lxc:sharenet type='netns' value='red'/ lxc:shareipc type='pid' value='12345'/ lxc:shareuts type='name' value='container1'/ /lxc:namespace The netns option is specific to sharenet. It can be used to inherit from existing network namespace. ACK and pushed to GIT master. Thanks for taking the time to work on this feature ! 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
[libvirt] [PATCH v4] lxc: Inherit namespace feature
This patch adds feature for lxc containers to inherit namespaces. This is very similar to what lxc-tools or docker provides. Look for man lxc-start and you will find that you can pass command args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker networking option in which you can give --net=container:NAME_or_ID as an option for sharing +namespace. From this patch you can add extra libvirt option to share namespace in following way. lxc:namespace lxc:sharenet type='netns' value='red'/ lxc:shareipc type='pid' value='12345'/ lxc:shareuts type='name' value='container1'/ /lxc:namespace The netns option is specific to sharenet. It can be used to inherit from existing network namespace. --- docs/drvlxc.html.in | 21 + docs/schemas/domaincommon.rng | 42 + po/POTFILES.in| 1 + src/Makefile.am | 7 +- src/lxc/lxc_conf.c| 2 +- src/lxc/lxc_container.c | 71 +-- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 57 +++- src/lxc/lxc_domain.c | 149 src/lxc/lxc_domain.h | 26 ++ src/lxc/lxc_process.c | 157 ++ tests/lxcxml2xmldata/lxc-sharenet.xml | 33 +++ tests/lxcxml2xmltest.c| 1 + 13 files changed, 560 insertions(+), 9 deletions(-) create mode 100644 tests/lxcxml2xmldata/lxc-sharenet.xml diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index a094bd9..d6c57c4 100644 --- a/docs/drvlxc.html.in +++ b/docs/drvlxc.html.in @@ -590,6 +590,27 @@ Note that allowing capabilities that are normally dropped by default can serious affect the security of the container and the host. /p +h2a name=shareInherit namespaces/a/h2 + +p +Libvirt allows you to inherit the namespace from container/process just like lxc tools +or docker provides to share the network namespace. The following can be used to share +required namespaces. If we want to share only one then the other namespaces can be ignored. +The netns option is specific to sharenet. It can be used in cases we want to use existing network namespace +rather than creating new network namespace for the container. In this case privnet option will be +ignored. +/p +pre +lt;domain type='lxc' xmlns:lxc='http://libvirt.org/schemas/domain/lxc/1.0'gt; +... +lt;lxc:namespacegt; + lt;lxc:sharenet type='netns' value='red'/gt; + lt;lxc:shareuts type='name' value='container1'/gt; + lt;lxc:shareipc type='pid' value='12345'/gt; +lt;/lxc:namespacegt; +lt;/domaingt; +/pre + h2a name=usageContainer usage / management/a/h2 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 043c975..fa026cd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -68,6 +68,9 @@ ref name='qemucmdline'/ /optional optional + ref name='lxcsharens'/ +/optional +optional ref name='keywrap'/ /optional /interleave @@ -5057,6 +5060,45 @@ /element /define + !-- + Optional hypervisor extensions in their own namespace: + LXC +-- + define name=lxcsharens +element name=namespace ns=http://libvirt.org/schemas/domain/lxc/1.0; + zeroOrMore +element name=sharenet + attribute name=type +choice + valuenetns/value + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element +element name=shareipc + attribute name=type +choice + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element +element name=shareuts + attribute name=type +choice + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element + /zeroOrMore +/element + /define + define name=metadata element name=metadata zeroOrMore diff --git a/po/POTFILES.in b/po/POTFILES.in index 1e52e6a..46220f7 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -85,6 +85,7 @@ src/lxc/lxc_native.c src/lxc/lxc_container.c src/lxc/lxc_conf.c src/lxc/lxc_controller.c +src/lxc/lxc_domain.c src/lxc/lxc_driver.c src/lxc/lxc_process.c src/libxl/libxl_domain.c diff --git a/src/Makefile.am b/src/Makefile.am index c4d49a5..24d31e1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1320,7 +1320,12 @@ libvirt_driver_lxc_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) -libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS) $(FUSE_LIBS) +libvirt_driver_lxc_impl_la_LIBADD =
Re: [libvirt] [PATCH v3] lxc: Inherit namespace feature
On 14.08.2015 14:09, Daniel P. Berrange wrote: From: Imran Khan ik.n...@gmail.com This patch adds feature for lxc containers to inherit namespaces. This is very similar to what lxc-tools or docker provides. Look for man lxc-start and you will find that you can pass command args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker networking option in which you can give --net=container:NAME_or_ID as an option for sharing +namespace. From this patch you can add extra libvirt option to share s/// namespace in following way. lxc:namespace lxc:sharenet type='netns' value='red'/ lxc:shareipc type='pid' value='12345'/ lxc:shareuts type='name' value='container1'/ /lxc:namespace The netns option is specific to sharenet. It can be used to inherit from existing network namespace. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/drvlxc.html.in | 21 ++ docs/schemas/domaincommon.rng | 42 po/POTFILES.in| 1 + src/Makefile.am | 6 +- src/lxc/lxc_conf.c| 2 +- src/lxc/lxc_container.c | 71 ++-- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 45 - src/lxc/lxc_domain.c | 149 ++ src/lxc/lxc_domain.h | 26 src/lxc/lxc_process.c | 149 ++ tests/lxcxml2xmltest.c| 1 + 12 files changed, 506 insertions(+), 9 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index e99b039..9699377 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -359,6 +359,135 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, return ret; } +static const char *nsInfoLocal[VIR_LXC_DOMAIN_NAMESPACE_LAST] = { +[VIR_LXC_DOMAIN_NAMESPACE_SHARENET] = net, +[VIR_LXC_DOMAIN_NAMESPACE_SHAREIPC] = ipc, +[VIR_LXC_DOMAIN_NAMESPACE_SHAREUTS] = uts, +}; + +static int virLXCProcessSetupNamespaceName(virConnectPtr conn, int ns_type, const char *name) +{ +virLXCDriverPtr driver = conn-privateData; +int fd = -1; +virDomainObjPtr vm; +char *path; + +vm = virDomainObjListFindByName(driver-domains, name); +if (!vm) { +virReportError(VIR_ERR_NO_DOMAIN, + _(No domain with matching name '%s'), name); +return -1; +} + +if (virAsprintf(path, /proc/%lld/ns/%s, +(long long int)vm-pid, +nsInfoLocal[ns_type]) 0) +goto cleanup; + +if ((fd = open(path, O_RDONLY)) 0) { +virReportSystemError(errno, + _(failed to open ns %s), + virLXCDomainNamespaceTypeToString(ns_type)); +goto cleanup; +} + + cleanup: +VIR_FREE(path); +virObjectUnlock(vm); +virObjectUnref(vm); +return fd; +} + + +static int virLXCProcessSetupNamespacePID(int ns_type, const char *name) +{ +int fd; +char *path; + +if (virAsprintf(path, /proc/%s/ns/%s, +name, +nsInfoLocal[ns_type]) 0) +return -1; +fd = open(path, O_RDONLY); +VIR_FREE(path); +if (fd 0) { +virReportSystemError(errno, + _(failed to open ns %s), + virLXCDomainNamespaceTypeToString(ns_type)); +return -1; +} +return fd; +} + + +static int virLXCProcessSetupNamespaceNet(int ns_type, const char *name) +{ +char *path; +int fd; +if (ns_type != VIR_LXC_DOMAIN_NAMESPACE_SHARENET) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s s/$/,/ + _('netns' namespace source can only be + used with sharenet)); +return -1; +} + +if (virAsprintf(path, /var/run/netns/%s, name) 0) +return -1; +fd = open(path, O_RDONLY); +VIR_FREE(path); +if (fd 0) { +virReportSystemError(errno, + _(failed to open netns %s), name); +return -1; +} +return fd; +} + + diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 3e00347..8d824b9 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -133,6 +133,7 @@ mymain(void) DO_TEST(filesystem-root); DO_TEST(idmap); DO_TEST(capabilities); +DO_TEST(sharenet); Have you forgot to git add tests/lxcxml2xmldata/lxc-sharenet.xml? I like the idea though. I'm tempted to ACK this if you fix all the small issues I've raised. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] lxc: Inherit namespace feature
From: Imran Khan ik.n...@gmail.com This patch adds feature for lxc containers to inherit namespaces. This is very similar to what lxc-tools or docker provides. Look for man lxc-start and you will find that you can pass command args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker networking option in which you can give --net=container:NAME_or_ID as an option for sharing +namespace. From this patch you can add extra libvirt option to share namespace in following way. lxc:namespace lxc:sharenet type='netns' value='red'/ lxc:shareipc type='pid' value='12345'/ lxc:shareuts type='name' value='container1'/ /lxc:namespace The netns option is specific to sharenet. It can be used to inherit from existing network namespace. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/drvlxc.html.in | 21 ++ docs/schemas/domaincommon.rng | 42 po/POTFILES.in| 1 + src/Makefile.am | 6 +- src/lxc/lxc_conf.c| 2 +- src/lxc/lxc_container.c | 71 ++-- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 45 - src/lxc/lxc_domain.c | 149 ++ src/lxc/lxc_domain.h | 26 src/lxc/lxc_process.c | 149 ++ tests/lxcxml2xmltest.c| 1 + 12 files changed, 506 insertions(+), 9 deletions(-) diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index a094bd9..d6c57c4 100644 --- a/docs/drvlxc.html.in +++ b/docs/drvlxc.html.in @@ -590,6 +590,27 @@ Note that allowing capabilities that are normally dropped by default can serious affect the security of the container and the host. /p +h2a name=shareInherit namespaces/a/h2 + +p +Libvirt allows you to inherit the namespace from container/process just like lxc tools +or docker provides to share the network namespace. The following can be used to share +required namespaces. If we want to share only one then the other namespaces can be ignored. +The netns option is specific to sharenet. It can be used in cases we want to use existing network namespace +rather than creating new network namespace for the container. In this case privnet option will be +ignored. +/p +pre +lt;domain type='lxc' xmlns:lxc='http://libvirt.org/schemas/domain/lxc/1.0'gt; +... +lt;lxc:namespacegt; + lt;lxc:sharenet type='netns' value='red'/gt; + lt;lxc:shareuts type='name' value='container1'/gt; + lt;lxc:shareipc type='pid' value='12345'/gt; +lt;/lxc:namespacegt; +lt;/domaingt; +/pre + h2a name=usageContainer usage / management/a/h2 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 043c975..fa026cd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -68,6 +68,9 @@ ref name='qemucmdline'/ /optional optional + ref name='lxcsharens'/ +/optional +optional ref name='keywrap'/ /optional /interleave @@ -5057,6 +5060,45 @@ /element /define + !-- + Optional hypervisor extensions in their own namespace: + LXC +-- + define name=lxcsharens +element name=namespace ns=http://libvirt.org/schemas/domain/lxc/1.0; + zeroOrMore +element name=sharenet + attribute name=type +choice + valuenetns/value + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element +element name=shareipc + attribute name=type +choice + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element +element name=shareuts + attribute name=type +choice + valuename/value + valuepid/value +/choice + /attribute + attribute name='value'/ +/element + /zeroOrMore +/element + /define + define name=metadata element name=metadata zeroOrMore diff --git a/po/POTFILES.in b/po/POTFILES.in index c58a7c1..dcabcc8 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -85,6 +85,7 @@ src/lxc/lxc_native.c src/lxc/lxc_container.c src/lxc/lxc_conf.c src/lxc/lxc_controller.c +src/lxc/lxc_domain.c src/lxc/lxc_driver.c src/lxc/lxc_process.c src/libxl/libxl_domain.c diff --git a/src/Makefile.am b/src/Makefile.am index c4d49a5..fde11ff 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1320,7 +1320,11 @@ libvirt_driver_lxc_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) -libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS) $(FUSE_LIBS) +libvirt_driver_lxc_impl_la_LIBADD = \ + $(CAPNG_LIBS) \ + $(LIBNL_LIBS) \ +
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. 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] Revert LXC: show used memory as 0 when domain is not active
On Tue, Aug 11, 2015 at 10:25:19AM +0200, Peter Krempa wrote: On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote: On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. 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] Revert LXC: show used memory as 0 when domain is not active
On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote: On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d8d5119..5b0a757 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -597,7 +597,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info-cpuTime = 0; -info-memory = 0; +info-memory = vm-def-mem.cur_balloon; } else { if (virCgroupGetCpuacctUsage(priv-cgroup, (info-cpuTime)) 0) { virReportError(VIR_ERR_OPERATION_FAILED, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6998e12..48cc534 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2641,13 +2641,13 @@ qemuDomainGetInfo(virDomainPtr dom, goto cleanup; } -if (virDomainObjIsActive(vm)) { -if (VIR_ASSIGN_IS_OVERFLOW(info-memory, vm-def-mem.cur_balloon)) { -virReportError(VIR_ERR_OVERFLOW, %s, - _(Current memory size too large)); -goto cleanup; -} +if (VIR_ASSIGN_IS_OVERFLOW(info-memory, vm-def-mem.cur_balloon)) { +virReportError(VIR_ERR_OVERFLOW, %s, + _(Current memory size too large)); +goto cleanup; +} +if (virDomainObjIsActive(vm)) { if (qemuGetProcessInfo((info-cpuTime), NULL, NULL, vm-pid, 0) 0) { virReportError(VIR_ERR_OPERATION_FAILED, %s, _(cannot read cputime for domain)); -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: don't up the veth interfaces unless explicitly asked to
On 24.04.2015 15:52, Lubomir Rintel wrote: Upping an interface for no reason and not configuring it is a cardinal sin. With the default addrgenmode if eui64 it sticks a link-local address to the interface. That is not good, as NetworkManager would see an address configured, assume the interface is already configured and won't touch it iself and the interface might stay unconfigured until the end of the days. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1124721 --- src/lxc/lxc_container.c | 2 +- src/lxc/lxc_native.c| 10 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index cc20b6d..bd135c7 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -541,7 +541,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, VIR_FREE(ipStr); } -if (netDef-linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { +if (netDef-nips || netDef-linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) { I'd split this long line into two. VIR_DEBUG(Enabling %s, newname); rc = virNetDevSetOnline(newname, true); if (rc 0) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index c15eb19..2297dbe 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -348,12 +348,10 @@ lxcCreateNetDef(const char *type, if (VIR_ALLOC(net) 0) goto error; -if (flag) { -if (STREQ(flag, up)) -net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; -else -net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; -} +if (flag STREQ(flag, up)) or STREQ_NULLABLE(). +net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; +else +net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; if (VIR_STRDUP(net-ifname_guest, name) 0) goto error; I've fixed both nits, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: don't up the veth interfaces unless explicitly asked to
Upping an interface for no reason and not configuring it is a cardinal sin. With the default addrgenmode if eui64 it sticks a link-local address to the interface. That is not good, as NetworkManager would see an address configured, assume the interface is already configured and won't touch it iself and the interface might stay unconfigured until the end of the days. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1124721 --- src/lxc/lxc_container.c | 2 +- src/lxc/lxc_native.c| 10 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index cc20b6d..bd135c7 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -541,7 +541,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, VIR_FREE(ipStr); } -if (netDef-linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { +if (netDef-nips || netDef-linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) { VIR_DEBUG(Enabling %s, newname); rc = virNetDevSetOnline(newname, true); if (rc 0) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index c15eb19..2297dbe 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -348,12 +348,10 @@ lxcCreateNetDef(const char *type, if (VIR_ALLOC(net) 0) goto error; -if (flag) { -if (STREQ(flag, up)) -net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; -else -net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; -} +if (flag STREQ(flag, up)) +net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; +else +net-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN; if (VIR_STRDUP(net-ifname_guest, name) 0) goto error; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] lxc: create the required directories upon driver start
Hi Lubomir, On Wed, 2015-04-08 at 19:16 +0200, Lubomir Rintel wrote: /var/run may reside on a tmpfs and we fail to create the PID file if /var/run/lxc does not exist. I would enhance the commit message with something like this: Since commit 0a8addc1, the lxc driver's state directory isn't automatically created before starting a domain. Now, the lxc driver makes sure the state directory exists when it initializes. [cbosdon...@suse.com: use cfg-stateDir instead of LXC_STATE_DIR] This line shouldn't be in the commit message: see my other comment. Signed-off-by: Lubomir Rintel lkund...@v3.sk You don't have to sign-off your patches. --- The changes with the previous version should go here: this way they won't appear in the git commit message. I'll push your patch with those changes. Thanks for your help. -- Cedric src/lxc/lxc_driver.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 245000d..8dfa686 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1648,6 +1648,13 @@ static int lxcStateInitialize(bool privileged, if (!(caps = virLXCDriverGetCapabilities(lxc_driver, false))) goto cleanup; +if (virFileMakePath(cfg-stateDir) 0) { +virReportSystemError(errno, + _(Failed to mkdir %s), + cfg-stateDir); +goto cleanup; +} + /* Get all the running persistent or transient configs first */ if (virDomainObjListLoadAllConfigs(lxc_driver-domains, cfg-stateDir, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] lxc: create the required directories upon driver start
/var/run may reside on a tmpfs and we fail to create the PID file if /var/run/lxc does not exist. [cbosdon...@suse.com: use cfg-stateDir instead of LXC_STATE_DIR] Signed-off-by: Lubomir Rintel lkund...@v3.sk --- src/lxc/lxc_driver.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 245000d..8dfa686 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1648,6 +1648,13 @@ static int lxcStateInitialize(bool privileged, if (!(caps = virLXCDriverGetCapabilities(lxc_driver, false))) goto cleanup; +if (virFileMakePath(cfg-stateDir) 0) { +virReportSystemError(errno, + _(Failed to mkdir %s), + cfg-stateDir); +goto cleanup; +} + /* Get all the running persistent or transient configs first */ if (virDomainObjListLoadAllConfigs(lxc_driver-domains, cfg-stateDir, -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] lxc: fix show the wrong xml when guest start failed
On 02/04/2015 08:42 AM, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1176503 When guest start failed, libvirt will keep the current vm-def, this will make a issue that we cannot get a right xml after guest start failed. And don't call the stop/release hook to do some other clean work. Call virLXCProcessCleanup to help us clean the source and call the hooks if start a vm failed Signed-off-by: Luyao Huang lhu...@redhat.com --- v2: use virLXCProcessCleanup to free the source and call the hook. v3: rework the patch to suit the virLXCProcessStart code changed. src/lxc/lxc_process.c | 76 ++- 1 file changed, 32 insertions(+), 44 deletions(-) FWIW: v1 review starts here: http://www.redhat.com/archives/libvir-list/2015-January/msg00112.html At first I was 'concerned' about the number of changes, but after doing more investigation - I think the patch is correct in general; however, I think it needs some adjustments as there are really two bugs here... I'll send an update shortly for comparison/review... Essentially though - I think the console check*s* could be done earlier before we get into other setup that requires going thru cleanup: (or what error: was). That's one bug - which is a configuration error which will prevent startup. Since it's easier to check early, provide an error, and return -1 - that's what I think is cleaner solution. Second, the other bug which you were trying to clean up at the same time is that existing cleanup paths don't properly clean all things up. The error path worked only when/once the container started and some pre-container start cleanup was done, but a few important ones were missing - so that's a separate patch. I also will add a patch to add/modify the debugging to help future such efforts... BTW: It seems commit id '88a1b542' from Cédric Bosdonnat caused yet another merge conflict and seemingly from the description might have been in the same area, but alas a quick check shows, it wasn't the same issue. I'll leave the notes I was developing on this patch prior to just biting the bullet and reformatting so you can perhaps see my thought process. diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 01da344..1a6cfbb 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1022,6 +1022,7 @@ int virLXCProcessStart(virConnectPtr conn, virCgroupPtr selfcgroup; int status; char *pidfile = NULL; +bool need_stop = false; I think the check: if (vm-def-nconsoles == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(At least one PTY console is required)); goto cleanup; } Should perhaps be moved to just before this code: nttyFDs = vm-def-nconsoles; if (VIR_ALLOC_N(ttyFDs, nttyFDs) 0) goto cleanup; for (i = 0; i vm-def-nconsoles; i++) ttyFDs[i] = -1; and that would fix the bug from the bz... as well as ensuring we don't have a if (VIR_ALLOC_N(ttdFDs, 0) 0)... In fact is there a reason why that check cannot move much higher and be after the cgroup checks which return -1? While t it, why not move the following too: for (i = 0; i vm-def-nconsoles; i++) { if (vm-def-consoles[i]-source.type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Only PTY console types are supported)); goto cleanup; } } and then remove that from the loop later on. This way checks that go to cleanup are a result of some error in processing rather than some container configuration error... Then the remainder of the code could be perhaps patch 2 which is fixing a different, although related problem. if (virCgroupNewSelf(selfcgroup) 0) return -1; @@ -1259,6 +1260,7 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } +need_stop = true; priv-stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; priv-wantReboot = false; vm-def-id = vm-pid; I was going to suggest using vm-pid (e.g. 0) instead of 'need_stop'; however, I couldn't convince myself that would be 'safe enough'... @@ -1272,20 +1274,20 @@ int virLXCProcessStart(virConnectPtr conn, if (VIR_CLOSE(handshakefds[1]) 0) { virReportSystemError(errno, %s, _(could not close handshake fd)); -goto error; +goto cleanup; } if (virCommandHandshakeWait(cmd) 0) -goto error; +goto cleanup; /* Write domain status to disk for the controller to * read when it starts */ if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) -goto error; +goto cleanup; /* Allow the child to exec the controller */ if (virCommandHandshakeNotify(cmd) 0) -goto error; +goto cleanup; if (virAtomicIntInc(driver-nactive) == 1
Re: [libvirt] [PATCH v3] lxc: fix show the wrong xml when guest start failed
On 02/13/2015 04:45 AM, John Ferlan wrote: On 02/04/2015 08:42 AM, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1176503 When guest start failed, libvirt will keep the current vm-def, this will make a issue that we cannot get a right xml after guest start failed. And don't call the stop/release hook to do some other clean work. Call virLXCProcessCleanup to help us clean the source and call the hooks if start a vm failed Signed-off-by: Luyao Huang lhu...@redhat.com --- v2: use virLXCProcessCleanup to free the source and call the hook. v3: rework the patch to suit the virLXCProcessStart code changed. src/lxc/lxc_process.c | 76 ++- 1 file changed, 32 insertions(+), 44 deletions(-) FWIW: v1 review starts here: http://www.redhat.com/archives/libvir-list/2015-January/msg00112.html At first I was 'concerned' about the number of changes, but after doing more investigation - I think the patch is correct in general; however, I think it needs some adjustments as there are really two bugs here... I'll send an update shortly for comparison/review... Yes, i agree about these patch need more adjustment, because i think maybe there is a better way to fix these issue but i cannot find them ;) and thanks for your patches. Essentially though - I think the console check*s* could be done earlier before we get into other setup that requires going thru cleanup: (or what error: was). That's one bug - which is a configuration error which will prevent startup. Since it's easier to check early, provide an error, and return -1 - that's what I think is cleaner solution. Looks good for me Second, the other bug which you were trying to clean up at the same time is that existing cleanup paths don't properly clean all things up. The error path worked only when/once the container started and some pre-container start cleanup was done, but a few important ones were missing - so that's a separate patch. I also will add a patch to add/modify the debugging to help future such efforts... Thanks BTW: It seems commit id '88a1b542' from Cédric Bosdonnat caused yet another merge conflict and seemingly from the description might have been in the same area, but alas a quick check shows, it wasn't the same issue. I'll leave the notes I was developing on this patch prior to just biting the bullet and reformatting so you can perhaps see my thought process. Yes, commit 88a1b542 is different from the issue i try to fix. diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 01da344..1a6cfbb 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1022,6 +1022,7 @@ int virLXCProcessStart(virConnectPtr conn, virCgroupPtr selfcgroup; int status; char *pidfile = NULL; +bool need_stop = false; I think the check: if (vm-def-nconsoles == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(At least one PTY console is required)); goto cleanup; } Should perhaps be moved to just before this code: nttyFDs = vm-def-nconsoles; if (VIR_ALLOC_N(ttyFDs, nttyFDs) 0) goto cleanup; for (i = 0; i vm-def-nconsoles; i++) ttyFDs[i] = -1; and that would fix the bug from the bz... as well as ensuring we don't have a if (VIR_ALLOC_N(ttdFDs, 0) 0)... In fact is there a reason why that check cannot move much higher and be after the cgroup checks which return -1? While t it, why not move the following too: for (i = 0; i vm-def-nconsoles; i++) { if (vm-def-consoles[i]-source.type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Only PTY console types are supported)); goto cleanup; } } and then remove that from the loop later on. Yes, after your words, i think console check in this place should be improved. This way checks that go to cleanup are a result of some error in processing rather than some container configuration error... Then the remainder of the code could be perhaps patch 2 which is fixing a different, although related problem. if (virCgroupNewSelf(selfcgroup) 0) return -1; @@ -1259,6 +1260,7 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } ... -VIR_FREE(vm-def-seclabels[0]-imagelabel); -VIR_DELETE_ELEMENT(vm-def-seclabels, 0, vm-def-nseclabels); +err = virSaveLastError(); +if (need_stop) { +virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); +} else { +virSecurityManagerRestoreAllLabel(driver-securityManager, + vm-def, false); +virSecurityManagerReleaseLabel(driver-securityManager, vm-def); +/* Clear out dynamically assigned labels */ +if (vm-def-nseclabels +vm-def-seclabels[0]-type ==
[libvirt] [PATCH v3] lxc: fix show the wrong xml when guest start failed
https://bugzilla.redhat.com/show_bug.cgi?id=1176503 When guest start failed, libvirt will keep the current vm-def, this will make a issue that we cannot get a right xml after guest start failed. And don't call the stop/release hook to do some other clean work. Call virLXCProcessCleanup to help us clean the source and call the hooks if start a vm failed Signed-off-by: Luyao Huang lhu...@redhat.com --- v2: use virLXCProcessCleanup to free the source and call the hook. v3: rework the patch to suit the virLXCProcessStart code changed. src/lxc/lxc_process.c | 76 ++- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 01da344..1a6cfbb 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1022,6 +1022,7 @@ int virLXCProcessStart(virConnectPtr conn, virCgroupPtr selfcgroup; int status; char *pidfile = NULL; +bool need_stop = false; if (virCgroupNewSelf(selfcgroup) 0) return -1; @@ -1259,6 +1260,7 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } +need_stop = true; priv-stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; priv-wantReboot = false; vm-def-id = vm-pid; @@ -1272,20 +1274,20 @@ int virLXCProcessStart(virConnectPtr conn, if (VIR_CLOSE(handshakefds[1]) 0) { virReportSystemError(errno, %s, _(could not close handshake fd)); -goto error; +goto cleanup; } if (virCommandHandshakeWait(cmd) 0) -goto error; +goto cleanup; /* Write domain status to disk for the controller to * read when it starts */ if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) -goto error; +goto cleanup; /* Allow the child to exec the controller */ if (virCommandHandshakeNotify(cmd) 0) -goto error; +goto cleanup; if (virAtomicIntInc(driver-nactive) == 1 driver-inhibitCallback) driver-inhibitCallback(true, driver-inhibitOpaque); @@ -1298,7 +1300,7 @@ int virLXCProcessStart(virConnectPtr conn, _(guest failed to start: %s), out); } -goto error; +goto cleanup; } /* We know the cgroup must exist by this synchronization @@ -1310,13 +1312,13 @@ int virLXCProcessStart(virConnectPtr conn, vm-def-resource-partition : NULL, -1, priv-cgroup) 0) -goto error; +goto cleanup; if (!priv-cgroup) { virReportError(VIR_ERR_INTERNAL_ERROR, _(No valid cgroup for machine %s), vm-def-name); -goto error; +goto cleanup; } /* And we can get the first monitor connection now too */ @@ -1329,17 +1331,17 @@ int virLXCProcessStart(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _(guest failed to start: %s), ebuf); } -goto error; +goto cleanup; } if (autoDestroy virCloseCallbacksSet(driver-closeCallbacks, vm, conn, lxcProcessAutoDestroy) 0) -goto error; +goto cleanup; if (virDomainObjSetDefTransient(caps, driver-xmlopt, vm, false) 0) -goto error; +goto cleanup; /* We don't need the temporary NIC names anymore, clear them */ virLXCProcessCleanInterfaces(vm-def); @@ -1358,47 +1360,38 @@ int virLXCProcessStart(virConnectPtr conn, * If the script raised an error abort the launch */ if (hookret 0) -goto error; +goto cleanup; } rc = 0; cleanup: -if (rc != 0 !err) -err = virSaveLastError(); -virCommandFree(cmd); if (VIR_CLOSE(logfd) 0) { virReportSystemError(errno, %s, _(could not close logfile)); rc = -1; } -for (i = 0; i nveths; i++) { -if (rc != 0 veths[i]) -ignore_value(virNetDevVethDelete(veths[i])); -VIR_FREE(veths[i]); -} if (rc != 0) { -if (vm-newDef) { -virDomainDefFree(vm-newDef); -vm-newDef = NULL; -} -if (priv-monitor) { -virObjectUnref(priv-monitor); -priv-monitor = NULL; -} -virDomainConfVMNWFilterTeardown(vm); - -virSecurityManagerRestoreAllLabel(driver-securityManager, - vm-def, false); -virSecurityManagerReleaseLabel(driver-securityManager, vm-def); -/* Clear out dynamically assigned labels */ -if (vm-def-nseclabels -vm-def-seclabels[0]-type == VIR_DOMAIN_SECLABEL_DYNAMIC) { -VIR_FREE(vm-def-seclabels[0]-model); -VIR_FREE(vm-def-seclabels[0]-label); -
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
-Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Monday, December 22, 2014 11:57 AM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
Am 24.12.2014 um 03:23 schrieb Chen, Hanxiao: -Original Message- From: Richard Weinberger [mailto:richard.weinber...@gmail.com] Sent: Wednesday, December 24, 2014 5:36 AM To: Eric Blake Cc: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com Subject: Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled On Mon, Dec 22, 2014 at 4:12 PM, Eric Blake ebl...@redhat.com wrote: On 12/21/2014 08:57 PM, Chen Hanxiao wrote: s/namespce/namespace/ in the subject line If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) I'll leave the actual patch review to someone more familiar with LXC namespace setups This change will still mount some useless stuff like: { /.oldroot/proc/sys/net/ipv4, /proc/sys/net/ipv4, NULL, MS_BIND, false, false, true }, { /.oldroot/proc/sys/net/ipv6, /proc/sys/net/ipv6, NULL, MS_BIND, false, false, true }, You can set skipUserNS for these. Thanks, I didn't notice that. But I *really* would like to see /proc and /sys mounted RW as default. Please see my comment to: [libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers I see your new comments in that thread. If libvirt enable userns(provided a uid/gid map in XML), it's safe to drop RO mount completely; If not, I'm not sure whether it will bring back compatibility issues. So let's wait for more comments from maintainers. I Agree Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
On Mon, Dec 22, 2014 at 4:12 PM, Eric Blake ebl...@redhat.com wrote: On 12/21/2014 08:57 PM, Chen Hanxiao wrote: s/namespce/namespace/ in the subject line If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) I'll leave the actual patch review to someone more familiar with LXC namespace setups This change will still mount some useless stuff like: { /.oldroot/proc/sys/net/ipv4, /proc/sys/net/ipv4, NULL, MS_BIND, false, false, true }, { /.oldroot/proc/sys/net/ipv6, /proc/sys/net/ipv6, NULL, MS_BIND, false, false, true }, You can set skipUserNS for these. But I *really* would like to see /proc and /sys mounted RW as default. Please see my comment to: [libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers -- Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
-Original Message- From: Richard Weinberger [mailto:richard.weinber...@gmail.com] Sent: Wednesday, December 24, 2014 5:36 AM To: Eric Blake Cc: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com Subject: Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled On Mon, Dec 22, 2014 at 4:12 PM, Eric Blake ebl...@redhat.com wrote: On 12/21/2014 08:57 PM, Chen Hanxiao wrote: s/namespce/namespace/ in the subject line If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) I'll leave the actual patch review to someone more familiar with LXC namespace setups This change will still mount some useless stuff like: { /.oldroot/proc/sys/net/ipv4, /proc/sys/net/ipv4, NULL, MS_BIND, false, false, true }, { /.oldroot/proc/sys/net/ipv6, /proc/sys/net/ipv6, NULL, MS_BIND, false, false, true }, You can set skipUserNS for these. Thanks, I didn't notice that. But I *really* would like to see /proc and /sys mounted RW as default. Please see my comment to: [libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers I see your new comments in that thread. If libvirt enable userns(provided a uid/gid map in XML), it's safe to drop RO mount completely; If not, I'm not sure whether it will bring back compatibility issues. So let's wait for more comments from maintainers. Regards, - Chen -- Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
On 12/21/2014 08:57 PM, Chen Hanxiao wrote: s/namespce/namespace/ in the subject line If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) I'll leave the actual patch review to someone more familiar with LXC namespace setups -- 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
[libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
If we enabled user ns and provided a uid/gid map, we do not need to mount /proc, /sys as readonly. Leave it to kernel for protection. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1b9e2f2..3b5845a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -983,6 +983,12 @@ static int lxcContainerMountBasicFS(bool userns_enabled, goto cleanup; } +/* don't readonly mount when userns is enabled */ +if (userns_enabled) { +VIR_FREE(mnt_src); +continue; +} + if (bindOverReadonly mount(mnt_src, mnt-dst, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) 0) { -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: add HOME environment variable
-Original Message- On Tue, Aug 12, 2014 at 11:21:41AM +0200, Richard Weinberger wrote: On Mon, Aug 11, 2014 at 11:13 AM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Aug 05, 2014 at 02:40:53AM +, chenhanx...@cn.fujitsu.com wrote: ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, July 25, 2014 2:40 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: add HOME environment variable We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); I'm curious what expects to have a $HOME env var set. I'd tend to view the setting of $HOME to be something that the software in the container should take care of. The kernel sets up $HOME for the init process. Therefore any init can assume that $HOME is set. libvirt currently violates that implicit rule. Ah ok, that makese sense then. ACK Could anyone help to push this patch according to Dan's ACK? Thanks, - Chen Setting HOME=/ in libvirt isn't a problem, I'm just curious why we need it. 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 RFC] LXC: add HOME environment variable
On 09/04/2014 03:58 AM, Chen, Hanxiao wrote: The kernel sets up $HOME for the init process. Therefore any init can assume that $HOME is set. libvirt currently violates that implicit rule. Ah ok, that makese sense then. ACK Could anyone help to push this patch according to Dan's ACK? Done, after amending the commit message to include Richard's summary of why it is important. -- 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] libvirt: lxc: Add Get/Set vcpus for lxc
On 2014/8/22 17:50, Li Yang wrote: 1.Add function to get vcpu count for lxc(vcpucount) 2.Add function to set vcpu count for lxc(setvcpus) Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com --- src/lxc/lxc_driver.c | 159 ++ 1 files changed, 159 insertions(+), 0 deletions(-) Does def-vcpus affect anything? No matter how much vcpus I set in xml , it seems that the vcpu count in container is equal to the host pcpu count. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libvirt: lxc: Add Get/Set vcpus for lxc
1.Add function to get vcpu count for lxc(vcpucount) 2.Add function to set vcpu count for lxc(setvcpus) Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com --- src/lxc/lxc_driver.c | 159 ++ 1 files changed, 159 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 4741632..4df0738 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5617,6 +5617,162 @@ lxcDomainGetMetadata(virDomainPtr dom, return ret; } +static int +lxcDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, +unsigned int flags) +{ +virLXCDriverPtr driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +virDomainDefPtr persistentDef; +int ret = -1; +bool maximum; +unsigned int maxvcpus = 0; +virLXCDriverConfigPtr cfg = NULL; +virCapsPtr caps = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_VCPU_MAXIMUM | + VIR_DOMAIN_VCPU_GUEST, -1); + +if (!nvcpus || (unsigned short) nvcpus != nvcpus) { +virReportError(VIR_ERR_INVALID_ARG, + _(argument out of range: %d), nvcpus); +return -1; +} + +if (!(vm = lxcDomObjFromDomain(dom))) +goto cleanup; + +cfg = virLXCDriverGetConfig(driver); + +if (virDomainSetVcpusFlagsEnsureACL(dom-conn, vm-def, flags) 0) +goto cleanup; + +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +maximum = (flags VIR_DOMAIN_VCPU_MAXIMUM) != 0; +flags = ~VIR_DOMAIN_VCPU_MAXIMUM; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) +goto cleanup; + +/* MAXIMUM cannot be mixed with LIVE. */ +if (maximum (flags VIR_DOMAIN_AFFECT_LIVE)) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(cannot adjust maximum on running domain)); +goto cleanup; +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) +maxvcpus = vm-def-maxvcpus; +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!maxvcpus || maxvcpus persistentDef-maxvcpus) +maxvcpus = persistentDef-maxvcpus; +} +if (!maximum nvcpus maxvcpus) { +virReportError(VIR_ERR_INVALID_ARG, + _(requested vcpus is greater than max allowable + vcpus for the domain: %d %d), + nvcpus, maxvcpus); +goto cleanup; +} + +if (flags VIR_DOMAIN_VCPU_GUEST) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(changing of vCPU count isn't supported + via guest agent)); +goto cleanup; +} else { +if (flags VIR_DOMAIN_AFFECT_LIVE) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(Cannot hotplug vCPUS for LXC hypervisor)); +goto cleanup; +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (maximum) { +persistentDef-maxvcpus = nvcpus; +if (nvcpus persistentDef-vcpus) +persistentDef-vcpus = nvcpus; +} else { +persistentDef-vcpus = nvcpus; +} + +if (virDomainSaveConfig(cfg-configDir, persistentDef) 0) +goto cleanup; +} +} + +ret = 0; + + cleanup: +if (vm) +virObjectUnlock(vm); +virObjectUnref(caps); +virObjectUnref(cfg); +return ret; +} + + +static int +lxcDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) +{ +return lxcDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_AFFECT_LIVE); +} + + +static int +lxcDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) +{ +virLXCDriverPtr driver = dom-conn-privateData; +virDomainObjPtr vm; +virDomainDefPtr def; +int ret = -1; +virCapsPtr caps = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_VCPU_MAXIMUM | + VIR_DOMAIN_VCPU_GUEST, -1); + +if (!(vm = lxcDomObjFromDomain(dom))) +return -1; + +if (virDomainGetVcpusFlagsEnsureACL(dom-conn, vm-def, flags) 0) +goto cleanup; + +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, +vm, flags, def) 0) +goto cleanup; + +if (flags VIR_DOMAIN_AFFECT_LIVE) +def = vm-def; + +if (flags VIR_DOMAIN_VCPU_GUEST) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(vCPU count cannot be provided by the guest agent + for LXC hypervisor)); +goto cleanup; +} else { +if (flags VIR_DOMAIN_VCPU_MAXIMUM) +
Re: [libvirt] [PATCH RFC] LXC: add HOME environment variable
On Mon, Aug 11, 2014 at 11:13 AM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Aug 05, 2014 at 02:40:53AM +, chenhanx...@cn.fujitsu.com wrote: ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, July 25, 2014 2:40 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: add HOME environment variable We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); I'm curious what expects to have a $HOME env var set. I'd tend to view the setting of $HOME to be something that the software in the container should take care of. The kernel sets up $HOME for the init process. Therefore any init can assume that $HOME is set. libvirt currently violates that implicit rule. Setting HOME=/ in libvirt isn't a problem, I'm just curious why we need it. 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 -- Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: add HOME environment variable
On Tue, Aug 12, 2014 at 11:21:41AM +0200, Richard Weinberger wrote: On Mon, Aug 11, 2014 at 11:13 AM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Aug 05, 2014 at 02:40:53AM +, chenhanx...@cn.fujitsu.com wrote: ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, July 25, 2014 2:40 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: add HOME environment variable We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); I'm curious what expects to have a $HOME env var set. I'd tend to view the setting of $HOME to be something that the software in the container should take care of. The kernel sets up $HOME for the init process. Therefore any init can assume that $HOME is set. libvirt currently violates that implicit rule. Ah ok, that makese sense then. ACK Setting HOME=/ in libvirt isn't a problem, I'm just curious why we need it. 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 RFC] LXC: add HOME environment variable
On Tue, Aug 05, 2014 at 02:40:53AM +, chenhanx...@cn.fujitsu.com wrote: ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, July 25, 2014 2:40 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: add HOME environment variable We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); I'm curious what expects to have a $HOME env var set. I'd tend to view the setting of $HOME to be something that the software in the container should take care of. Setting HOME=/ in libvirt isn't a problem, I'm just curious why we need it. 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 RFC] LXC: add HOME environment variable
On Fri, Jul 25, 2014 at 8:39 AM, Chen Hanxiao chenhanx...@cn.fujitsu.com wrote: We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); Looks sane to me. Reviewed-by: Richard Weinberger rich...@nod.at -- Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] LXC: add HOME environment variable
ping -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Chen Hanxiao Sent: Friday, July 25, 2014 2:40 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH RFC] LXC: add HOME environment variable We lacked of HOME environment variable, set 'HOME=/' as default. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..9df9c04 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -236,6 +236,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); virCommandAddEnvString(cmd, container=lxc-libvirt); +virCommandAddEnvString(cmd, HOME=/); virCommandAddEnvPair(cmd, container_uuid, uuidstr); if (nttyPaths 1) virCommandAddEnvPair(cmd, container_ttys, virBufferCurrentContent(buf)); -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3] Rework lxc apparmor profile
Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- Diff to v2: * Fixed missing goto cleanup examples/apparmor/Makefile.am | 6 +- examples/apparmor/TEMPLATE.lxc| 15 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +- examples/apparmor/libvirt-lxc | 119 +++--- src/security/security_apparmor.c | 21 +++-- src/security/virt-aa-helper.c | 29 +-- 6 files changed, 149 insertions(+), 43 deletions(-) create mode 100644 examples/apparmor/TEMPLATE.lxc rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index a741e94..7a20e16 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -15,7 +15,8 @@ ## http://www.gnu.org/licenses/. EXTRA_DIST=\ - TEMPLATE\ + TEMPLATE.qemu \ + TEMPLATE.lxc\ libvirt-qemu\ libvirt-lxc \ usr.lib.libvirt.virt-aa-helper \ @@ -36,6 +37,7 @@ abstractions_DATA = \ templatesdir = $(apparmordir)/libvirt templates_DATA = \ - TEMPLATE \ + TEMPLATE.qemu \ + TEMPLATE.lxc \ $(NULL) endif WITH_APPARMOR_PROFILES diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc new file mode 100644 index 000..7b64885 --- /dev/null +++ b/examples/apparmor/TEMPLATE.lxc @@ -0,0 +1,15 @@ +# +# This profile is for the domain whose UUID matches this file. +# + +#include tunables/global + +profile LIBVIRT_TEMPLATE { + #include abstractions/libvirt-lxc + + # Globally allows everything to run under this profile + # These can be narrowed depending on the container's use. + file, + capability, + network, +} diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu similarity index 75% rename from examples/apparmor/TEMPLATE rename to examples/apparmor/TEMPLATE.qemu index 187dec5..008a221 100644 --- a/examples/apparmor/TEMPLATE +++ b/examples/apparmor/TEMPLATE.qemu @@ -5,5 +5,5 @@ #include tunables/global profile LIBVIRT_TEMPLATE { - #include abstractions/libvirt-driver + #include abstractions/libvirt-qemu } diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ #include abstractions/base - # Needed for lxc-enter-namespace - capability sys_admin, - capability sys_chroot, + umount, - # Added for lxc-enter-namespace --cmd /bin/bash - /bin/bash PUx, + # ignore DENIED message on / remount + deny mount options=(ro, remount) - /, - /usr/sbin/cron PUx, - /usr/lib/systemd/systemd PUx, + # allow tmpfs mounts everywhere + mount fstype=tmpfs, - /usr/lib/libsystemd-*.so.* mr, - /usr/lib/libudev-*.so.* mr, - /etc/ld.so.cache mr, + # allow mqueue mounts everywhere + mount fstype=mqueue, + + # allow fuse mounts everywhere + mount fstype=fuse.*, + + # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted + mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/, + deny @{PROC}/sys/fs/** wklx, + + # allow efivars to be mounted, writing to it will be blocked though + mount fstype=efivarfs - /sys/firmware/efi/efivars/, + + # block some other dangerous paths + deny @{PROC}/sysrq-trigger rwklx, + deny @{PROC}/mem rwklx, + deny @{PROC}/kmem rwklx, + + # deny writes in /sys except for /sys/fs/cgroup, also allow + # fusectl, securityfs and debugfs to be mounted there (read-only) + mount fstype=fusectl - /sys/fs/fuse/connections/, + mount fstype=securityfs - /sys/kernel/security/, + mount fstype=debugfs - /sys/kernel/debug/, + mount fstype=proc - /proc/, + mount fstype=sysfs - /sys/, + deny /sys/firmware/efi/efivars/** rwklx, + deny /sys/kernel/security/** rwklx, + + # generated by: lxc-generate-aa-rules.py container-rules.base + deny /proc/sys/[^kn]*{,/**} wklx, + deny /proc/sys/k[^e]*{,/**} wklx, + deny /proc/sys/ke[^r]*{,/**} wklx, + deny /proc/sys/ker[^n]*{,/**} wklx, + deny /proc/sys/kern[^e]*{,/**} wklx, + deny /proc/sys/kerne[^l]*{,/**} wklx, + deny /proc/sys/kernel/[^smhd]*{,/**} wklx, + deny /proc/sys/kernel/d[^o]*{,/**} wklx, + deny /proc/sys/kernel/do[^m]*{,/**} wklx, + deny /proc/sys/kernel/dom[^a]*{,/**} wklx, + deny /proc/sys/kernel/doma[^i]*{,/**} wklx
Re: [libvirt] [PATCHv3] Rework lxc apparmor profile
On 07/15/2014 03:02 AM, Cédric Bosdonnat wrote: Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- Diff to v2: * Fixed missing goto cleanup Will push shortly, based on the ack given here: https://www.redhat.com/archives/libvir-list/2014-July/msg00745.html -- 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
[libvirt] [PATCH v3] LXC: add support for --config in setmem command
In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v3: add max_balloon check for AFFECT_CONFIG v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 58 ++-- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3253211..f04b543 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -711,36 +711,64 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) return ret; } -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { virDomainObjPtr vm; +virDomainDefPtr persistentDef = NULL; +virCapsPtr caps = NULL; int ret = -1; virLXCDomainObjPrivatePtr priv; +virLXCDriverPtr driver = dom-conn-privateData; +virLXCDriverConfigPtr cfg = NULL; +unsigned long oldmax = 0; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; +cfg = virLXCDriverGetConfig(driver); + priv = vm-privateData; -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def) 0) +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags) 0) goto cleanup; -if (newmem vm-def-mem.max_balloon) { +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) +goto cleanup; + +if (flags VIR_DOMAIN_AFFECT_LIVE) +oldmax = vm-def-mem.max_balloon; +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!oldmax || oldmax persistentDef-mem.max_balloon) +oldmax = persistentDef-mem.max_balloon; +} + +if (newmem oldmax) { virReportError(VIR_ERR_INVALID_ARG, %s, _(Cannot set memory higher than max memory)); goto cleanup; } -if (!virDomainObjIsActive(vm)) { -virReportError(VIR_ERR_OPERATION_INVALID, - %s, _(Domain is not running)); -goto cleanup; +if (flags VIR_DOMAIN_AFFECT_LIVE) { +if (virCgroupSetMemory(priv-cgroup, newmem) 0) { +virReportError(VIR_ERR_OPERATION_FAILED, + %s, _(Failed to set memory for domain)); +goto cleanup; +} } -if (virCgroupSetMemory(priv-cgroup, newmem) 0) { -virReportError(VIR_ERR_OPERATION_FAILED, - %s, _(Failed to set memory for domain)); -goto cleanup; +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +persistentDef-mem.cur_balloon = newmem; +if (virDomainSaveConfig(cfg-configDir, persistentDef) 0) +goto cleanup; } ret = 0; @@ -748,9 +776,16 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) cleanup: if (vm) virObjectUnlock(vm); +virObjectUnref(caps); +virObjectUnref(cfg); return ret; } +static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +{ +return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); +} + static int lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr params, @@ -5697,6 +5732,7 @@ static virDriver lxcDriver = { .domainGetMaxMemory = lxcDomainGetMaxMemory, /* 0.7.2 */ .domainSetMaxMemory = lxcDomainSetMaxMemory, /* 0.7.2 */ .domainSetMemory = lxcDomainSetMemory, /* 0.7.2 */ +.domainSetMemoryFlags = lxcDomainSetMemoryFlags, /* 1.2.7 */ .domainSetMemoryParameters = lxcDomainSetMemoryParameters, /* 0.8.5 */ .domainGetMemoryParameters = lxcDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = lxcDomainSetBlkioParameters, /* 0.9.8 */ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] LXC: add support for --config in setmem command
On 07/11/2014 11:26 AM, Chen Hanxiao wrote: In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v3: add max_balloon check for AFFECT_CONFIG v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 58 ++-- 1 file changed, 47 insertions(+), 11 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
--- src/security/virt-aa-helper.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..c8f17f9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,10 +1342,13 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; +} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) { +rc = 0; } else if ((rc = update_include_file(include_file, included_files, - ctl-append)) != 0) + ctl-append)) != 0) { goto cleanup; +} /* create the profile from TEMPLATE */ -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..d563b98 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,7 +1342,8 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; -} else if ((rc = update_include_file(include_file, +} else if (ctl-def-virtType != VIR_DOMAIN_VIRT_LXC + (rc = update_include_file(include_file, included_files, ctl-append)) != 0) goto cleanup; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On 07/11/2014 09:22 AM, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com I've pushed this one. -- 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 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On Fri, 2014-07-11 at 11:03 -0600, Eric Blake wrote: On 07/11/2014 09:22 AM, Serge Hallyn wrote: Quoting Cédric Bosdonnat (cbosdon...@suse.com): --- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Hi, I'm acking this anyway bc I think you're right, but I'm trying to think of a case where this would still be useful. What if we want to allow only a certain container to have access to its cgroups, for instance, for nesting containers. Would virt-aa-helper and the .files be a way this would be done? I suppose we coudl always re-introduce this in that case... Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com I've pushed this one. Huh, I found a regression with this one... sent a v2 earlier today. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
On 07/11/2014 07:01 AM, Cédric Bosdonnat wrote: --- src/security/virt-aa-helper.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..c8f17f9 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,10 +1342,13 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; +} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) { +rc = 0; } else if ((rc = update_include_file(include_file, included_files, - ctl-append)) != 0) + ctl-append)) != 0) { goto cleanup; +} I squashed this on top of a revert of v1, since I had pushed that before realizing you had posted a v2, and pushed the result. -- 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] LXC: add support for --config in setmem command
On 07/08/2014 10:32 AM, Chen Hanxiao wrote: In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 48 +--- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b47ac5e..93f496b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -711,18 +711,33 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) return ret; } -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { virDomainObjPtr vm; +virDomainDefPtr persistentDef = NULL; +virCapsPtr caps = NULL; int ret = -1; virLXCDomainObjPrivatePtr priv; +virLXCDriverPtr driver = dom-conn-privateData; +virLXCDriverConfigPtr cfg = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; priv = vm-privateData; -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def) 0) +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags) 0) +goto cleanup; + +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) goto cleanup; if (newmem vm-def-mem.max_balloon) { This check should only be done for AFFECT_LIVE. For AFFECT_CONFIG it needs to be checked against the max_balloon value from the persistent definition. Jan 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] LXC: add support for --config in setmem command
-Original Message- From: Ján Tomko [mailto:jto...@redhat.com] Sent: Thursday, July 10, 2014 9:40 PM -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def) 0) +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags) 0) +goto cleanup; + +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) goto cleanup; if (newmem vm-def-mem.max_balloon) { This check should only be done for AFFECT_LIVE. For AFFECT_CONFIG it needs to be checked against the max_balloon value from the persistent definition. Oops, my fault. Thanks for your comments - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles
--- src/security/virt-aa-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..d563b98 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1342,7 +1342,8 @@ main(int argc, char **argv) vah_info(include_file); vah_info(included_files); rc = 0; -} else if ((rc = update_include_file(include_file, +} else if (ctl-def-virtType != VIR_DOMAIN_VIRT_LXC + (rc = update_include_file(include_file, included_files, ctl-append)) != 0) goto cleanup; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] LXC: add support for --config in setmem command
In lxc, we could not use setmem command with --config options. This patch will add support for this. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: use virDomainSetMemoryFlagsEnsureACL remove redundant domain running check src/lxc/lxc_driver.c | 48 +--- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b47ac5e..93f496b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -711,18 +711,33 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) return ret; } -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, + unsigned int flags) { virDomainObjPtr vm; +virDomainDefPtr persistentDef = NULL; +virCapsPtr caps = NULL; int ret = -1; virLXCDomainObjPrivatePtr priv; +virLXCDriverPtr driver = dom-conn-privateData; +virLXCDriverConfigPtr cfg = NULL; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; priv = vm-privateData; -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def) 0) +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags) 0) +goto cleanup; + +if (!(caps = virLXCDriverGetCapabilities(driver, false))) +goto cleanup; + +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags, +persistentDef) 0) goto cleanup; if (newmem vm-def-mem.max_balloon) { @@ -731,16 +746,19 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) goto cleanup; } -if (!virDomainObjIsActive(vm)) { -virReportError(VIR_ERR_OPERATION_INVALID, - %s, _(Domain is not running)); -goto cleanup; -} + if (flags VIR_DOMAIN_AFFECT_CONFIG) { + cfg = virLXCDriverGetConfig(driver); + persistentDef-mem.cur_balloon = newmem; + if (virDomainSaveConfig(cfg-configDir, persistentDef) 0) + goto cleanup; + } -if (virCgroupSetMemory(priv-cgroup, newmem) 0) { -virReportError(VIR_ERR_OPERATION_FAILED, - %s, _(Failed to set memory for domain)); -goto cleanup; +if (flags VIR_DOMAIN_AFFECT_LIVE) { +if (virCgroupSetMemory(priv-cgroup, newmem) 0) { +virReportError(VIR_ERR_OPERATION_FAILED, + %s, _(Failed to set memory for domain)); +goto cleanup; +} } ret = 0; @@ -748,9 +766,16 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) cleanup: if (vm) virObjectUnlock(vm); +virObjectUnref(caps); +virObjectUnref(cfg); return ret; } +static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +{ +return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); +} + static int lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr params, @@ -5697,6 +5722,7 @@ static virDriver lxcDriver = { .domainGetMaxMemory = lxcDomainGetMaxMemory, /* 0.7.2 */ .domainSetMaxMemory = lxcDomainSetMaxMemory, /* 0.7.2 */ .domainSetMemory = lxcDomainSetMemory, /* 0.7.2 */ +.domainSetMemoryFlags = lxcDomainSetMemoryFlags, /* 1.2.7 */ .domainSetMemoryParameters = lxcDomainSetMemoryParameters, /* 0.8.5 */ .domainGetMemoryParameters = lxcDomainGetMemoryParameters, /* 0.8.5 */ .domainSetBlkioParameters = lxcDomainSetBlkioParameters, /* 0.9.8 */ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-lxc-convert: make free return values in bytes
On 07/04/2014 03:58 PM, Cédric Bosdonnat wrote: Tiny fix for virt-lxc-convert: we are setting memory values in bytes, while free may give us values in a different unit by default: force free to output bytes with -b flag. --- examples/lxcconvert/virt-lxc-convert | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK; pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-lxc-convert: make free return values in bytes
Tiny fix for virt-lxc-convert: we are setting memory values in bytes, while free may give us values in a different unit by default: force free to output bytes with -b flag. --- examples/lxcconvert/virt-lxc-convert | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/lxcconvert/virt-lxc-convert b/examples/lxcconvert/virt-lxc-convert index 7220153..e62172e 100644 --- a/examples/lxcconvert/virt-lxc-convert +++ b/examples/lxcconvert/virt-lxc-convert @@ -64,7 +64,7 @@ if test -r $fstab; then sed 's/^\([^#]\)/lxc.mount.entry = \1/' $fstab ${conf_new} fi -memory=$(free | sed -n '/Mem:/s/ \+/ /gp' | cut -f 2 -d ' ') +memory=$(free -b | sed -n '/Mem:/s/ \+/ /gp' | cut -f 2 -d ' ') default_tmpfs=size=$((memory/2)) # Do we have tmpfs without size param? -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: also fix a similar issue in lxcDomainGetSchedulerParametersFlags src/lxc/lxc_driver.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..d8a31d3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG, + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1993,7 +1999,13 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
On 06/24/2014 09:24 AM, Chen Hanxiao wrote: fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: also fix a similar issue in lxcDomainGetSchedulerParametersFlags src/lxc/lxc_driver.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..d8a31d3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG, + VIR_TYPED_PARAM_STRING_OKAY, -1); This fails to compile for me: CC lxc/libvirt_driver_lxc_impl_la-lxc_driver.lo lxc/lxc_driver.c:899:48: error: too many arguments provided to function-like macro invocation VIR_TYPED_PARAM_STRING_OKAY, -1); ^ + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ The original comment was OK. This is incorrect - neither of these APIs return a string. +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1993,7 +1999,13 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ Same here. +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; Jan 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] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
-Original Message- From: Ján Tomko [mailto:jto...@redhat.com] Sent: Tuesday, June 24, 2014 4:57 PM To: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com Subject: Re: [libvirt] [PATCH v2] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING On 06/24/2014 09:24 AM, Chen Hanxiao wrote: fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: also fix a similar issue in lxcDomainGetSchedulerParametersFlags src/lxc/lxc_driver.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..d8a31d3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG, + VIR_TYPED_PARAM_STRING_OKAY, -1); This fails to compile for me: CC lxc/libvirt_driver_lxc_impl_la-lxc_driver.lo lxc/lxc_driver.c:899:48: error: too many arguments provided to function-like macro invocation VIR_TYPED_PARAM_STRING_OKAY, -1); ^ Sorry for my mistake. + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ The original comment was OK. This is incorrect - neither of these APIs return a string. Thanks, I see. +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1993,7 +1999,13 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ Same here. +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: also fix a similar issue in lxcDomainGetSchedulerParametersFlags v3: fix improper comments src/lxc/lxc_driver.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 06f3e18..3875bf3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -895,7 +895,11 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We don't return strings, and thus trivially support this flag. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -1993,7 +1997,11 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + +/* We don't return strings, and thus trivially support this flag. */ +flags = ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] LXC: trivially support flag VIR_DRV_FEATURE_TYPED_PARAM_STRING
On 06/24/2014 11:44 AM, Chen Hanxiao wrote: fix: virsh -c lxc:/// memtune DOMAIN error: Unable to get number of memory parameters error: unsupported flags (0x4) in function lxcDomainGetMemoryParameters Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- v2: also fix a similar issue in lxcDomainGetSchedulerParametersFlags v3: fix improper comments src/lxc/lxc_driver.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list