Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---

Notes:
    new in v2

 src/conf/domain_conf.c          | 46 +++++++++++++++++++----------------------
 src/conf/domain_conf.h          |  9 ++++----
 src/security/security_dac.c     | 26 ++++++++++-------------
 src/security/security_manager.c |  4 ++--
 src/security/security_selinux.c | 24 +++++++++------------
 5 files changed, 49 insertions(+), 60 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7e20b8ba1..68dc2832cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 
 void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
 {
+    size_t i;
+
     if (!def)
         return;
 
     virDomainChrSourceDefClear(def);
     virObjectUnref(def->privateData);
 
+    if (def->seclabels) {
+        for (i = 0; i < def->nseclabels; i++)
+            virSecurityDeviceLabelDefFree(def->seclabels[i]);
+        VIR_FREE(def->seclabels);
+    }
+
+
     VIR_FREE(def);
 }
 
@@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
 
 void virDomainChrDefFree(virDomainChrDefPtr def)
 {
-    size_t i;
-
     if (!def)
         return;
 
@@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
     virDomainChrSourceDefFree(def->source);
     virDomainDeviceInfoClear(&def->info);
 
-    if (def->seclabels) {
-        for (i = 0; i < def->nseclabels; i++)
-            virSecurityDeviceLabelDefFree(def->seclabels[i]);
-        VIR_FREE(def->seclabels);
-    }
-
     VIR_FREE(def);
 }
 
@@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
                 if (chr_def) {
                     xmlNodePtr saved_node = ctxt->node;
                     ctxt->node = cur;
-                    if (virSecurityDeviceLabelDefParseXML(&chr_def->seclabels,
-                                                          &chr_def->nseclabels,
+                    if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
+                                                          &def->nseclabels,
                                                           vmSeclabels,
                                                           nvmSeclabels,
                                                           ctxt,
@@ -22399,19 +22400,11 @@ virDomainNetDefFormat(virBufferPtr buf,
  * output at " type='type'>". */
 static int
 virDomainChrSourceDefFormat(virBufferPtr buf,
-                            virDomainChrDefPtr chr_def,
                             virDomainChrSourceDefPtr def,
                             bool tty_compat,
                             unsigned int flags)
 {
     const char *type = virDomainChrTypeToString(def->type);
-    size_t nseclabels = 0;
-    virSecurityDeviceLabelDefPtr *seclabels = NULL;
-
-    if (chr_def) {
-        nseclabels = chr_def->nseclabels;
-        seclabels = chr_def->seclabels;
-    }
 
     if (!type) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -22449,7 +22442,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
                 def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT)
                 virBufferAsprintf(buf, " append='%s'",
                     virTristateSwitchTypeToString(def->data.file.append));
-            virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
+            virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
+                                             def->seclabels, flags);
         }
         break;
 
@@ -22504,7 +22498,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
             virBufferAsprintf(buf, "<source mode='%s'",
                               def->data.nix.listen ? "bind" : "connect");
             virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
-            virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
+            virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
+                                             def->seclabels, flags);
         }
         break;
 
@@ -22553,7 +22548,7 @@ virDomainChrDefFormat(virBufferPtr buf,
                   def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
                   !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
                   def->source->data.file.path);
-    if (virDomainChrSourceDefFormat(buf, def, def->source, tty_compat, flags) < 0)
+    if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
         return -1;
 
     /* Format <target> block */
@@ -22675,7 +22670,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
         break;
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        if (virDomainChrSourceDefFormat(buf, NULL, def->data.passthru, false,
+        if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
                                         flags) < 0)
             return -1;
         break;
@@ -22981,7 +22976,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
 
     case VIR_DOMAIN_RNG_BACKEND_EGD:
         virBufferAdjustIndent(buf, 2);
-        if (virDomainChrSourceDefFormat(buf, NULL, def->source.chardev,
+        if (virDomainChrSourceDefFormat(buf, def->source.chardev,
                                         false, flags) < 0)
             return -1;
         virBufferAdjustIndent(buf, -2);
@@ -23797,7 +23792,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
 
     virBufferAsprintf(buf, "<redirdev bus='%s'", bus);
     virBufferAdjustIndent(buf, 2);
-    if (virDomainChrSourceDefFormat(buf, NULL, def->source, false, flags) < 0)
+    if (virDomainChrSourceDefFormat(buf, def->source, false, flags) < 0)
         return -1;
     if (virDomainDeviceInfoFormat(buf, &def->info,
                                   flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0)
@@ -26195,7 +26190,8 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
 
 
 virSecurityDeviceLabelDefPtr
-virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
+virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
+                                         const char *model)
 {
     size_t i;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 83e0672691..1951ba74bb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1166,6 +1166,9 @@ struct _virDomainChrSourceDef {
     } data;
     char *logfile;
     int logappend;
+
+    size_t nseclabels;
+    virSecurityDeviceLabelDefPtr *seclabels;
 };
 
 /* A complete character device, both host and domain views.  */
@@ -1188,9 +1191,6 @@ struct _virDomainChrDef {
     virDomainChrSourceDefPtr source;
 
     virDomainDeviceInfo info;
-
-    size_t nseclabels;
-    virSecurityDeviceLabelDefPtr *seclabels;
 };
 
 typedef enum {
@@ -3068,7 +3068,8 @@ virSecurityLabelDefPtr
 virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
 
 virSecurityDeviceLabelDefPtr
-virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
+virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
+                                         const char *model);
 
 typedef const char* (*virEventActionToStringFunc)(int type);
 typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 7dcf4c15f7..fd4d8f5047 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1159,7 +1159,6 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
                               virDomainDefPtr def,
-                              virDomainChrDefPtr dev,
                               virDomainChrSourceDefPtr dev_source)
 
 {
@@ -1173,9 +1172,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 
     seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
 
-    if (dev)
-        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
-                                                          SECURITY_DAC_NAME);
+    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
+                                                            SECURITY_DAC_NAME);
 
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
@@ -1245,7 +1243,6 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 static int
 virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr def ATTRIBUTE_UNUSED,
-                                  virDomainChrDefPtr dev,
                                   virDomainChrSourceDefPtr dev_source)
 {
     virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
@@ -1253,9 +1250,8 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
     char *in = NULL, *out = NULL;
     int ret = -1;
 
-    if (dev)
-        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
-                                                          SECURITY_DAC_NAME);
+    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
+                                                            SECURITY_DAC_NAME);
 
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
@@ -1304,12 +1300,12 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
 
 static int
 virSecurityDACRestoreChardevCallback(virDomainDefPtr def,
-                                     virDomainChrDefPtr dev,
+                                     virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                      void *opaque)
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecurityDACRestoreChardevLabel(mgr, def, dev, dev->source);
+    return virSecurityDACRestoreChardevLabel(mgr, def, dev->source);
 }
 
 
@@ -1322,7 +1318,7 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
 
     switch (tpm->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-        ret = virSecurityDACSetChardevLabel(mgr, def, NULL,
+        ret = virSecurityDACSetChardevLabel(mgr, def,
                                             &tpm->data.passthrough.source);
         break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
@@ -1342,8 +1338,8 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
 
     switch (tpm->type) {
     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-        ret = virSecurityDACRestoreChardevLabel(mgr, def, NULL,
-                                          &tpm->data.passthrough.source);
+        ret = virSecurityDACRestoreChardevLabel(mgr, def,
+                                                &tpm->data.passthrough.source);
         break;
     case VIR_DOMAIN_TPM_TYPE_LAST:
         break;
@@ -1506,12 +1502,12 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
 
 static int
 virSecurityDACSetChardevCallback(virDomainDefPtr def,
-                                 virDomainChrDefPtr dev,
+                                 virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                  void *opaque)
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecurityDACSetChardevLabel(mgr, def, dev, dev->source);
+    return virSecurityDACSetChardevLabel(mgr, def, dev->source);
 }
 
 
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 6c777db1e6..90d491c1bc 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -811,8 +811,8 @@ virSecurityManagerCheckChardevLabel(virSecurityManagerPtr mgr,
 {
     size_t i;
 
-    for (i = 0; i < dev->nseclabels; i++) {
-        if (virSecurityManagerCheckModel(mgr, dev->seclabels[i]->model) < 0)
+    for (i = 0; i < dev->source->nseclabels; i++) {
+        if (virSecurityManagerCheckModel(mgr, dev->source->seclabels[i]->model) < 0)
             return -1;
     }
 
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 9504a4be34..75f387b3fa 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2179,7 +2179,6 @@ virSecuritySELinuxRestoreHostdevLabel(virSecurityManagerPtr mgr,
 static int
 virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr def,
-                                  virDomainChrDefPtr dev,
                                   virDomainChrSourceDefPtr dev_source)
 
 {
@@ -2193,9 +2192,8 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
     if (!seclabel || !seclabel->relabel)
         return 0;
 
-    if (dev)
-        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
-                                                          SECURITY_SELINUX_NAME);
+    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
+                                                            SECURITY_SELINUX_NAME);
 
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
@@ -2254,7 +2252,6 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
 static int
 virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr def,
-                                      virDomainChrDefPtr dev,
                                       virDomainChrSourceDefPtr dev_source)
 
 {
@@ -2267,9 +2264,8 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
     if (!seclabel || !seclabel->relabel)
         return 0;
 
-    if (dev)
-        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
-                                                          SECURITY_SELINUX_NAME);
+    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
+                                                            SECURITY_SELINUX_NAME);
     if (chr_seclabel && !chr_seclabel->relabel)
         return 0;
 
@@ -2318,12 +2314,12 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
 
 static int
 virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
-                                                 virDomainChrDefPtr dev,
+                                                 virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                                  void *opaque)
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev, dev->source);
+    return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->source);
 }
 
 
@@ -2346,7 +2342,7 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
         return virSecuritySELinuxRestoreFileLabel(mgr, database);
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        return virSecuritySELinuxRestoreChardevLabel(mgr, def, NULL, dev->data.passthru);
+        return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->data.passthru);
 
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2707,12 +2703,12 @@ virSecuritySELinuxClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
 
 static int
 virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
-                                             virDomainChrDefPtr dev,
+                                             virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
                                              void *opaque)
 {
     virSecurityManagerPtr mgr = opaque;
 
-    return virSecuritySELinuxSetChardevLabel(mgr, def, dev, dev->source);
+    return virSecuritySELinuxSetChardevLabel(mgr, def, dev->source);
 }
 
 
@@ -2736,7 +2732,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
         return virSecuritySELinuxSetFilecon(mgr, database, data->content_context);
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        return virSecuritySELinuxSetChardevLabel(mgr, def, NULL,
+        return virSecuritySELinuxSetChardevLabel(mgr, def,
                                                  dev->data.passthru);
 
     default:
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> 
> Notes:
>     new in v2
> 
>  src/conf/domain_conf.c          | 46 +++++++++++++++++++----------------------
>  src/conf/domain_conf.h          |  9 ++++----
>  src/security/security_dac.c     | 26 ++++++++++-------------
>  src/security/security_manager.c |  4 ++--
>  src/security/security_selinux.c | 24 +++++++++------------
>  5 files changed, 49 insertions(+), 60 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c7e20b8ba1..68dc2832cb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>  
>  void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
>  {
> +    size_t i;
> +
>      if (!def)
>          return;
>  
>      virDomainChrSourceDefClear(def);
>      virObjectUnref(def->privateData);
>  
> +    if (def->seclabels) {
> +        for (i = 0; i < def->nseclabels; i++)
> +            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> +        VIR_FREE(def->seclabels);
> +    }
> +
> +
>      VIR_FREE(def);
>  }
>  
> @@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
>  
>  void virDomainChrDefFree(virDomainChrDefPtr def)
>  {
> -    size_t i;
> -
>      if (!def)
>          return;
>  
> @@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
>      virDomainChrSourceDefFree(def->source);
>      virDomainDeviceInfoClear(&def->info);
>  
> -    if (def->seclabels) {
> -        for (i = 0; i < def->nseclabels; i++)
> -            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> -        VIR_FREE(def->seclabels);
> -    }
> -
>      VIR_FREE(def);
>  }
>  
> @@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>                  if (chr_def) {

Is the @chr_def check necessary still?  I assume it's there because
chr_def can be passed as NULL in some cases.

Looks like all this was added as part of commit 'f8b08d0e'

The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which
each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that
leads me to believe that the @chr_def should be removed.

The rest looks good, so

Reviewed-by: John Ferlan <jferlan@redhat.com>


John

>                      xmlNodePtr saved_node = ctxt->node;
>                      ctxt->node = cur;
> -                    if (virSecurityDeviceLabelDefParseXML(&chr_def->seclabels,
> -                                                          &chr_def->nseclabels,
> +                    if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
> +                                                          &def->nseclabels,
>                                                            vmSeclabels,
>                                                            nvmSeclabels,
>                                                            ctxt,
> @@ -22399,19 +22400,11 @@ virDomainNetDefFormat(virBufferPtr buf,
>   * output at " type='type'>". */
>  static int
>  virDomainChrSourceDefFormat(virBufferPtr buf,
> -                            virDomainChrDefPtr chr_def,
>                              virDomainChrSourceDefPtr def,
>                              bool tty_compat,
>                              unsigned int flags)
>  {
>      const char *type = virDomainChrTypeToString(def->type);
> -    size_t nseclabels = 0;
> -    virSecurityDeviceLabelDefPtr *seclabels = NULL;
> -
> -    if (chr_def) {
> -        nseclabels = chr_def->nseclabels;
> -        seclabels = chr_def->seclabels;
> -    }
>  
>      if (!type) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -22449,7 +22442,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
>                  def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT)
>                  virBufferAsprintf(buf, " append='%s'",
>                      virTristateSwitchTypeToString(def->data.file.append));
> -            virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
> +            virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
> +                                             def->seclabels, flags);
>          }
>          break;
>  
> @@ -22504,7 +22498,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
>              virBufferAsprintf(buf, "<source mode='%s'",
>                                def->data.nix.listen ? "bind" : "connect");
>              virBufferEscapeString(buf, " path='%s'", def->data.nix.path);
> -            virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);
> +            virDomainSourceDefFormatSeclabel(buf, def->nseclabels,
> +                                             def->seclabels, flags);
>          }
>          break;
>  
> @@ -22553,7 +22548,7 @@ virDomainChrDefFormat(virBufferPtr buf,
>                    def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
>                    !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
>                    def->source->data.file.path);
> -    if (virDomainChrSourceDefFormat(buf, def, def->source, tty_compat, flags) < 0)
> +    if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
>          return -1;
>  
>      /* Format <target> block */
> @@ -22675,7 +22670,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
>          break;
>  
>      case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -        if (virDomainChrSourceDefFormat(buf, NULL, def->data.passthru, false,
> +        if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
>                                          flags) < 0)
>              return -1;
>          break;
> @@ -22981,7 +22976,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
>  
>      case VIR_DOMAIN_RNG_BACKEND_EGD:
>          virBufferAdjustIndent(buf, 2);
> -        if (virDomainChrSourceDefFormat(buf, NULL, def->source.chardev,
> +        if (virDomainChrSourceDefFormat(buf, def->source.chardev,
>                                          false, flags) < 0)
>              return -1;
>          virBufferAdjustIndent(buf, -2);
> @@ -23797,7 +23792,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
>  
>      virBufferAsprintf(buf, "<redirdev bus='%s'", bus);
>      virBufferAdjustIndent(buf, 2);
> -    if (virDomainChrSourceDefFormat(buf, NULL, def->source, false, flags) < 0)
> +    if (virDomainChrSourceDefFormat(buf, def->source, false, flags) < 0)
>          return -1;
>      if (virDomainDeviceInfoFormat(buf, &def->info,
>                                    flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0)
> @@ -26195,7 +26190,8 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
>  
>  
>  virSecurityDeviceLabelDefPtr
> -virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
> +virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
> +                                         const char *model)
>  {
>      size_t i;
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 83e0672691..1951ba74bb 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1166,6 +1166,9 @@ struct _virDomainChrSourceDef {
>      } data;
>      char *logfile;
>      int logappend;
> +
> +    size_t nseclabels;
> +    virSecurityDeviceLabelDefPtr *seclabels;
>  };
>  
>  /* A complete character device, both host and domain views.  */
> @@ -1188,9 +1191,6 @@ struct _virDomainChrDef {
>      virDomainChrSourceDefPtr source;
>  
>      virDomainDeviceInfo info;
> -
> -    size_t nseclabels;
> -    virSecurityDeviceLabelDefPtr *seclabels;
>  };
>  
>  typedef enum {
> @@ -3068,7 +3068,8 @@ virSecurityLabelDefPtr
>  virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
>  
>  virSecurityDeviceLabelDefPtr
> -virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
> +virDomainChrSourceDefGetSecurityLabelDef(virDomainChrSourceDefPtr def,
> +                                         const char *model);
>  
>  typedef const char* (*virEventActionToStringFunc)(int type);
>  typedef int (*virEventActionFromStringFunc)(const char *type);
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 7dcf4c15f7..fd4d8f5047 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1159,7 +1159,6 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
>  static int
>  virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
>                                virDomainDefPtr def,
> -                              virDomainChrDefPtr dev,
>                                virDomainChrSourceDefPtr dev_source)
>  
>  {
> @@ -1173,9 +1172,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
>  
>      seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
>  
> -    if (dev)
> -        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> -                                                          SECURITY_DAC_NAME);
> +    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> +                                                            SECURITY_DAC_NAME);
>  
>      if (chr_seclabel && !chr_seclabel->relabel)
>          return 0;
> @@ -1245,7 +1243,6 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
>  static int
>  virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
>                                    virDomainDefPtr def ATTRIBUTE_UNUSED,
> -                                  virDomainChrDefPtr dev,
>                                    virDomainChrSourceDefPtr dev_source)
>  {
>      virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> @@ -1253,9 +1250,8 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
>      char *in = NULL, *out = NULL;
>      int ret = -1;
>  
> -    if (dev)
> -        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> -                                                          SECURITY_DAC_NAME);
> +    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> +                                                            SECURITY_DAC_NAME);
>  
>      if (chr_seclabel && !chr_seclabel->relabel)
>          return 0;
> @@ -1304,12 +1300,12 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
>  
>  static int
>  virSecurityDACRestoreChardevCallback(virDomainDefPtr def,
> -                                     virDomainChrDefPtr dev,
> +                                     virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
>                                       void *opaque)
>  {
>      virSecurityManagerPtr mgr = opaque;
>  
> -    return virSecurityDACRestoreChardevLabel(mgr, def, dev, dev->source);
> +    return virSecurityDACRestoreChardevLabel(mgr, def, dev->source);
>  }
>  
>  
> @@ -1322,7 +1318,7 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
>  
>      switch (tpm->type) {
>      case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> -        ret = virSecurityDACSetChardevLabel(mgr, def, NULL,
> +        ret = virSecurityDACSetChardevLabel(mgr, def,
>                                              &tpm->data.passthrough.source);
>          break;
>      case VIR_DOMAIN_TPM_TYPE_LAST:
> @@ -1342,8 +1338,8 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr,
>  
>      switch (tpm->type) {
>      case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> -        ret = virSecurityDACRestoreChardevLabel(mgr, def, NULL,
> -                                          &tpm->data.passthrough.source);
> +        ret = virSecurityDACRestoreChardevLabel(mgr, def,
> +                                                &tpm->data.passthrough.source);
>          break;
>      case VIR_DOMAIN_TPM_TYPE_LAST:
>          break;
> @@ -1506,12 +1502,12 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
>  
>  static int
>  virSecurityDACSetChardevCallback(virDomainDefPtr def,
> -                                 virDomainChrDefPtr dev,
> +                                 virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
>                                   void *opaque)
>  {
>      virSecurityManagerPtr mgr = opaque;
>  
> -    return virSecurityDACSetChardevLabel(mgr, def, dev, dev->source);
> +    return virSecurityDACSetChardevLabel(mgr, def, dev->source);
>  }
>  
>  
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 6c777db1e6..90d491c1bc 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -811,8 +811,8 @@ virSecurityManagerCheckChardevLabel(virSecurityManagerPtr mgr,
>  {
>      size_t i;
>  
> -    for (i = 0; i < dev->nseclabels; i++) {
> -        if (virSecurityManagerCheckModel(mgr, dev->seclabels[i]->model) < 0)
> +    for (i = 0; i < dev->source->nseclabels; i++) {
> +        if (virSecurityManagerCheckModel(mgr, dev->source->seclabels[i]->model) < 0)
>              return -1;
>      }
>  
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 9504a4be34..75f387b3fa 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2179,7 +2179,6 @@ virSecuritySELinuxRestoreHostdevLabel(virSecurityManagerPtr mgr,
>  static int
>  virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
>                                    virDomainDefPtr def,
> -                                  virDomainChrDefPtr dev,
>                                    virDomainChrSourceDefPtr dev_source)
>  
>  {
> @@ -2193,9 +2192,8 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
>      if (!seclabel || !seclabel->relabel)
>          return 0;
>  
> -    if (dev)
> -        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> -                                                          SECURITY_SELINUX_NAME);
> +    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> +                                                            SECURITY_SELINUX_NAME);
>  
>      if (chr_seclabel && !chr_seclabel->relabel)
>          return 0;
> @@ -2254,7 +2252,6 @@ virSecuritySELinuxSetChardevLabel(virSecurityManagerPtr mgr,
>  static int
>  virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>                                        virDomainDefPtr def,
> -                                      virDomainChrDefPtr dev,
>                                        virDomainChrSourceDefPtr dev_source)
>  
>  {
> @@ -2267,9 +2264,8 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>      if (!seclabel || !seclabel->relabel)
>          return 0;
>  
> -    if (dev)
> -        chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> -                                                          SECURITY_SELINUX_NAME);
> +    chr_seclabel = virDomainChrSourceDefGetSecurityLabelDef(dev_source,
> +                                                            SECURITY_SELINUX_NAME);
>      if (chr_seclabel && !chr_seclabel->relabel)
>          return 0;
>  
> @@ -2318,12 +2314,12 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>  
>  static int
>  virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
> -                                                 virDomainChrDefPtr dev,
> +                                                 virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
>                                                   void *opaque)
>  {
>      virSecurityManagerPtr mgr = opaque;
>  
> -    return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev, dev->source);
> +    return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->source);
>  }
>  
>  
> @@ -2346,7 +2342,7 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
>          return virSecuritySELinuxRestoreFileLabel(mgr, database);
>  
>      case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -        return virSecuritySELinuxRestoreChardevLabel(mgr, def, NULL, dev->data.passthru);
> +        return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->data.passthru);
>  
>      default:
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2707,12 +2703,12 @@ virSecuritySELinuxClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>  
>  static int
>  virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
> -                                             virDomainChrDefPtr dev,
> +                                             virDomainChrDefPtr dev ATTRIBUTE_UNUSED,
>                                               void *opaque)
>  {
>      virSecurityManagerPtr mgr = opaque;
>  
> -    return virSecuritySELinuxSetChardevLabel(mgr, def, dev, dev->source);
> +    return virSecuritySELinuxSetChardevLabel(mgr, def, dev->source);
>  }
>  
>  
> @@ -2736,7 +2732,7 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def,
>          return virSecuritySELinuxSetFilecon(mgr, database, data->content_context);
>  
>      case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -        return virSecuritySELinuxSetChardevLabel(mgr, def, NULL,
> +        return virSecuritySELinuxSetChardevLabel(mgr, def,
>                                                   dev->data.passthru);
>  
>      default:
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Jun 13, 2017 at 12:35:10PM -0400, John Ferlan wrote:
> 
> 
> On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> > 
> > Notes:
> >     new in v2
> > 
> >  src/conf/domain_conf.c          | 46 +++++++++++++++++++----------------------
> >  src/conf/domain_conf.h          |  9 ++++----
> >  src/security/security_dac.c     | 26 ++++++++++-------------
> >  src/security/security_manager.c |  4 ++--
> >  src/security/security_selinux.c | 24 +++++++++------------
> >  5 files changed, 49 insertions(+), 60 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index c7e20b8ba1..68dc2832cb 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
> >  
> >  void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
> >  {
> > +    size_t i;
> > +
> >      if (!def)
> >          return;
> >  
> >      virDomainChrSourceDefClear(def);
> >      virObjectUnref(def->privateData);
> >  
> > +    if (def->seclabels) {
> > +        for (i = 0; i < def->nseclabels; i++)
> > +            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> > +        VIR_FREE(def->seclabels);
> > +    }
> > +
> > +
> >      VIR_FREE(def);
> >  }
> >  
> > @@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
> >  
> >  void virDomainChrDefFree(virDomainChrDefPtr def)
> >  {
> > -    size_t i;
> > -
> >      if (!def)
> >          return;
> >  
> > @@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
> >      virDomainChrSourceDefFree(def->source);
> >      virDomainDeviceInfoClear(&def->info);
> >  
> > -    if (def->seclabels) {
> > -        for (i = 0; i < def->nseclabels; i++)
> > -            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> > -        VIR_FREE(def->seclabels);
> > -    }
> > -
> >      VIR_FREE(def);
> >  }
> >  
> > @@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> >                  if (chr_def) {
> 
> Is the @chr_def check necessary still?  I assume it's there because
> chr_def can be passed as NULL in some cases.
> 
> Looks like all this was added as part of commit 'f8b08d0e'
> 
> The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which
> each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that
> leads me to believe that the @chr_def should be removed.

But this hunk is from virDomainChrSourceDefParseXML() function.

> The rest looks good, so
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>

Thanks

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/15/2017 02:39 AM, Pavel Hrdina wrote:
> On Tue, Jun 13, 2017 at 12:35:10PM -0400, John Ferlan wrote:
>>
>>
>> On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> ---
>>>
>>> Notes:
>>>     new in v2
>>>
>>>  src/conf/domain_conf.c          | 46 +++++++++++++++++++----------------------
>>>  src/conf/domain_conf.h          |  9 ++++----
>>>  src/security/security_dac.c     | 26 ++++++++++-------------
>>>  src/security/security_manager.c |  4 ++--
>>>  src/security/security_selinux.c | 24 +++++++++------------
>>>  5 files changed, 49 insertions(+), 60 deletions(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index c7e20b8ba1..68dc2832cb 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>>>  
>>>  void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
>>>  {
>>> +    size_t i;
>>> +
>>>      if (!def)
>>>          return;
>>>  
>>>      virDomainChrSourceDefClear(def);
>>>      virObjectUnref(def->privateData);
>>>  
>>> +    if (def->seclabels) {
>>> +        for (i = 0; i < def->nseclabels; i++)
>>> +            virSecurityDeviceLabelDefFree(def->seclabels[i]);
>>> +        VIR_FREE(def->seclabels);
>>> +    }
>>> +
>>> +
>>>      VIR_FREE(def);
>>>  }
>>>  
>>> @@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
>>>  
>>>  void virDomainChrDefFree(virDomainChrDefPtr def)
>>>  {
>>> -    size_t i;
>>> -
>>>      if (!def)
>>>          return;
>>>  
>>> @@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
>>>      virDomainChrSourceDefFree(def->source);
>>>      virDomainDeviceInfoClear(&def->info);
>>>  
>>> -    if (def->seclabels) {
>>> -        for (i = 0; i < def->nseclabels; i++)
>>> -            virSecurityDeviceLabelDefFree(def->seclabels[i]);
>>> -        VIR_FREE(def->seclabels);
>>> -    }
>>> -
>>>      VIR_FREE(def);
>>>  }
>>>  
>>> @@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>>>                  if (chr_def) {
>>
>> Is the @chr_def check necessary still?  I assume it's there because
>> chr_def can be passed as NULL in some cases.
>>
>> Looks like all this was added as part of commit 'f8b08d0e'
>>
>> The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which
>> each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that
>> leads me to believe that the @chr_def should be removed.
> 
> But this hunk is from virDomainChrSourceDefParseXML() function.
> 

Yes, I knew that when I wrote the comment, but that wasn't the point.

Since you've moved the labels into @def instead and similarly altered
calls to virDomainChrSourceDefFormat such that they don't receive
chr_def (in the very next hunk of changes BTW), then I would think at
this point removing chr_def would be safe, but I suppose I could be
wrong. Hence why I asked.

So does it hurt to keep it, probably not; however, IIUC the reason why
it was there was because if it wasn't, then dereferencing chr_def to get
the [n]seclabels in the subsequent call wouldn't end well.

John

>> The rest looks good, so
>>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> Thanks
> 
> Pavel
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jun 15, 2017 at 06:50:34AM -0400, John Ferlan wrote:
> 
> 
> On 06/15/2017 02:39 AM, Pavel Hrdina wrote:
> > On Tue, Jun 13, 2017 at 12:35:10PM -0400, John Ferlan wrote:
> >>
> >>
> >> On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
> >>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> >>> ---
> >>>
> >>> Notes:
> >>>     new in v2
> >>>
> >>>  src/conf/domain_conf.c          | 46 +++++++++++++++++++----------------------
> >>>  src/conf/domain_conf.h          |  9 ++++----
> >>>  src/security/security_dac.c     | 26 ++++++++++-------------
> >>>  src/security/security_manager.c |  4 ++--
> >>>  src/security/security_selinux.c | 24 +++++++++------------
> >>>  5 files changed, 49 insertions(+), 60 deletions(-)
> >>>
> >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >>> index c7e20b8ba1..68dc2832cb 100644
> >>> --- a/src/conf/domain_conf.c
> >>> +++ b/src/conf/domain_conf.c
> >>> @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
> >>>  
> >>>  void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def)
> >>>  {
> >>> +    size_t i;
> >>> +
> >>>      if (!def)
> >>>          return;
> >>>  
> >>>      virDomainChrSourceDefClear(def);
> >>>      virObjectUnref(def->privateData);
> >>>  
> >>> +    if (def->seclabels) {
> >>> +        for (i = 0; i < def->nseclabels; i++)
> >>> +            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> >>> +        VIR_FREE(def->seclabels);
> >>> +    }
> >>> +
> >>> +
> >>>      VIR_FREE(def);
> >>>  }
> >>>  
> >>> @@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
> >>>  
> >>>  void virDomainChrDefFree(virDomainChrDefPtr def)
> >>>  {
> >>> -    size_t i;
> >>> -
> >>>      if (!def)
> >>>          return;
> >>>  
> >>> @@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
> >>>      virDomainChrSourceDefFree(def->source);
> >>>      virDomainDeviceInfoClear(&def->info);
> >>>  
> >>> -    if (def->seclabels) {
> >>> -        for (i = 0; i < def->nseclabels; i++)
> >>> -            virSecurityDeviceLabelDefFree(def->seclabels[i]);
> >>> -        VIR_FREE(def->seclabels);
> >>> -    }
> >>> -
> >>>      VIR_FREE(def);
> >>>  }
> >>>  
> >>> @@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> >>>                  if (chr_def) {
> >>
> >> Is the @chr_def check necessary still?  I assume it's there because
> >> chr_def can be passed as NULL in some cases.
> >>
> >> Looks like all this was added as part of commit 'f8b08d0e'
> >>
> >> The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which
> >> each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that
> >> leads me to believe that the @chr_def should be removed.
> > 
> > But this hunk is from virDomainChrSourceDefParseXML() function.
> > 
> 
> Yes, I knew that when I wrote the comment, but that wasn't the point.
> 
> Since you've moved the labels into @def instead and similarly altered
> calls to virDomainChrSourceDefFormat such that they don't receive
> chr_def (in the very next hunk of changes BTW), then I would think at
> this point removing chr_def would be safe, but I suppose I could be
> wrong. Hence why I asked.
> 
> So does it hurt to keep it, probably not; however, IIUC the reason why
> it was there was because if it wasn't, then dereferencing chr_def to get
> the [n]seclabels in the subsequent call wouldn't end well.

Oh right.  The only thing what that check currently does is that for
smartcard, rng and redirdev it skips parsing of the seclabel.  I would
probably leave it to a separate patch which would ensure that we don't
start parsing the seclabel for these devices.

Pavel

> 
> John
> 
> >> The rest looks good, so
> >>
> >> Reviewed-by: John Ferlan <jferlan@redhat.com>
> > 
> > Thanks
> > 
> > Pavel
> > 
> 
> --
> 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