Re: [libvirt] [PATCH v2] lxc: Include support to lxc version 3.0 or higher.

2018-11-06 Thread 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.

2018-11-05 Thread 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.

2018-11-05 Thread 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