11:30 a.m.
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 <jcfaracco(a)gmail.com>
---
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", &value) <= 0)
- return -1;
+ if (virConfGetValueString(properties, "lxc.rootfs.path", &value) <=
0) {
+ virResetLastError();
+
+ /* Check for pre LXC 3.0 legacy key */
+ if (virConfGetValueString(properties, "lxc.rootfs", &value) <=
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", &value) <= 0)
- return 0;
+ if (virConfGetValueString(properties, "lxc.tty.max", &value) <= 0)
{
+ virResetLastError();
+
+ /* Check for pre LXC 3.0 legacy key */
+ if (virConfGetValueString(properties, "lxc.tty", &value) <= 0)
+ return 0;
+ }
if (virStrToLong_i(value, NULL, 10, &nbttys) < 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", &type,
&target, &start, &count) != 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", &value) <= 0
||
- VIR_STRDUP(vmdef->name, value) < 0)
- goto error;
+ if (virConfGetValueString(properties, "lxc.uts.name", &value) <= 0)
{
+ virResetLastError();
+
+ /* Check for pre LXC 3.0 legacy key */
+ if (virConfGetValueString(properties, "lxc.utsname", &value) <=
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
7:17 a.m.
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 <jcfaracco(a)gmail.com>
---
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", &type,
&target, &start, &count) != 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", &value) <= 0
||
- VIR_STRDUP(vmdef->name, value) < 0)
- goto error;
+ if (virConfGetValueString(properties, "lxc.uts.name", &value) <= 0)
{
+ virResetLastError();
+
+ /* Check for pre LXC 3.0 legacy key */
+ if (virConfGetValueString(properties, "lxc.utsname", &value) <=
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 <jferlan(a)redhat.com>
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.
[...]
11:46 a.m.
Em sáb, 10 de nov de 2018 às 11:17, John Ferlan <jferlan(a)redhat.com> 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 <jcfaracco(a)gmail.com>
> ---
> 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", &type,
> &target, &start, &count) != 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", &value)
<= 0 ||
> - VIR_STRDUP(vmdef->name, value) < 0)
> - goto error;
> + if (virConfGetValueString(properties, "lxc.uts.name", &value)
<= 0) {
> + virResetLastError();
> +
> + /* Check for pre LXC 3.0 legacy key */
> + if (virConfGetValueString(properties, "lxc.utsname", &value)
<= 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 <jferlan(a)redhat.com>
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.
[...]
11:09 a.m.
On 11/11/18 12:46 PM, Julio Faracco wrote:
Em sáb, 10 de nov de 2018 às 11:17, John Ferlan
<jferlan(a)redhat.com> 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 <jcfaracco(a)gmail.com>
>> ---
>> 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", &type,
>> &target, &start, &count) != 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", &value)
<= 0 ||
>> - VIR_STRDUP(vmdef->name, value) < 0)
>> - goto error;
>> + if (virConfGetValueString(properties, "lxc.uts.name", &value)
<= 0) {
>> + virResetLastError();
>> +
>> + /* Check for pre LXC 3.0 legacy key */
>> + if (virConfGetValueString(properties, "lxc.utsname",
&value) <= 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 <jferlan(a)redhat.com>
>
> 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
5:13 a.m.
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 <jcfaracco(a)gmail.com>
---
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 :|
8:48 a.m.
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 <jcfaracco(a)gmail.com>
> ---
> 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)
2202
days inactive
2205
days old
5 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
John Ferlan -
Julio Faracco -
Laine Stump