Re: [libvirt] [PATCH v2] lxc: Refresh capabilities if they have never been initalized

[Daniel P . Berrangé]

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

[Cole Robinson]

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

[Michal Privoznik]

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"

[Daniel P . Berrangé]

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.

[John Ferlan]

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.

[Laine Stump]

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.

[Daniel P . Berrangé]

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.

[Julio Faracco]

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.

[John Ferlan]

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.

[Julio Faracco]

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.

[Martin Kletzander]

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.

[John Ferlan]

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.

[Julio Faracco]

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.

[John Ferlan]

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.

[Pino Toscano]

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.

[Julio Faracco]

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.

[Michal Privoznik]

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.

[Julio Faracco]

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.

[Julio Faracco]

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.

[Julio Faracco]

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

[Benjamin Cama]

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

[Benjamin Cama]

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

[Michal Privoznik]

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

[Cédric Bosdonnat]

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

[Daniel P. Berrange]

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

[Vasiliy Tolstov]

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

[Serge Hallyn]

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

[Serge Hallyn]

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

[Daniel P. Berrange]

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

[Serge Hallyn]

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

[Serge Hallyn]

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

[Daniel P. Berrange]

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

[Cedric Bosdonnat]

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

[Fabio Kung]

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.

--
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

[Serge Hallyn]

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

[Daniel P. Berrange]

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

[Serge Hallyn]

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

[Fabio Kung]

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

[Daniel P. Berrange]

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

[Serge Hallyn]

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

[Serge Hallyn]

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

[Andrea Bolognani]

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

[Daniel P. Berrange]

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

[Cedric Bosdonnat]

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

[Cédric Bosdonnat]

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

[Daniel P. Berrange]

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

[Jim Fehlig]

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

[Jim Fehlig]

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

[Eric Blake]

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

[John Ferlan]

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

[Daniel P. Berrange]

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

[ik.nitk]

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

[Michal Privoznik]

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

[Daniel P. Berrange]

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

[Daniel P. Berrange]

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

[Daniel P. Berrange]

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

[Peter Krempa]

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

[Jim Fehlig]

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

[Michal Privoznik]

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

[Lubomir Rintel]

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

[Cedric Bosdonnat]

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

[Lubomir Rintel]

/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

[John Ferlan]

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

[lhuang]

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

[Luyao Huang]

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

[Chen, Hanxiao]

 -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

[Richard Weinberger]

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

[Richard Weinberger]

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

[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.

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

[Eric Blake]

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

[Chen Hanxiao]

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

[Chen, Hanxiao]

 -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

[Eric Blake]

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

[Wang Rui]

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

[Li Yang]

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

[Richard Weinberger]

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

[Daniel P. Berrange]

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

[Daniel P. Berrange]

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

[Richard Weinberger]

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

[chenhanx...@cn.fujitsu.com]

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

[Cédric Bosdonnat]

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

[Eric Blake]

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

[Chen Hanxiao]

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

[Ján Tomko]

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

[Cédric Bosdonnat]

---
 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

[Serge Hallyn]

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

[Eric Blake]

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

[Cedric Bosdonnat]

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

[Eric Blake]

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

[Ján Tomko]

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

[chenhanx...@cn.fujitsu.com]

 -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

[Cédric Bosdonnat]

---
 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

[Chen Hanxiao]

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

[Ján Tomko]

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

[Cédric Bosdonnat]

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

[Chen Hanxiao]

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

[Ján Tomko]

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

[chenhanx...@cn.fujitsu.com]

 -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

[Chen Hanxiao]

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

[Ján Tomko]

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