This will be needed by future patches adding appid API to allow changing
it for running VMs.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_validate.c | 42 +++++++++++++++++++++++---------------
 src/conf/domain_validate.h |  2 ++
 src/libvirt_private.syms   |  1 +
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 1bc62c364d..3ab864bbeb 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -58,29 +58,37 @@ virDomainDefBootValidate(const virDomainDef *def)
 #define APPID_LEN_MIN 1
 #define APPID_LEN_MAX 128
 
+int
+virDomainDefResourceAppidValidate(const char *appid)
+{
+    int len;
+
+    if (!virStringIsPrintable(appid)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Fibre Channel 'appid' is not a printable string"));
+        return -1;
+    }
+
+    len = strlen(appid);
+    if (len < APPID_LEN_MIN || len > APPID_LEN_MAX) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("Fibre Channel 'appid' string length must be between [%d, %d]"),
+                       APPID_LEN_MIN, APPID_LEN_MAX);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainDefResourceValidate(const virDomainDef *def)
 {
     if (!def->resource)
         return 0;
 
-    if (def->resource->appid) {
-        int len;
-
-        if (!virStringIsPrintable(def->resource->appid)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Fibre Channel 'appid' is not a printable string"));
-            return -1;
-        }
-
-        len = strlen(def->resource->appid);
-        if (len < APPID_LEN_MIN || len > APPID_LEN_MAX) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("Fibre Channel 'appid' string length must be between [%d, %d]"),
-                           APPID_LEN_MIN, APPID_LEN_MAX);
-            return -1;
-        }
-    }
+    if (def->resource->appid)
+        return virDomainDefResourceAppidValidate(def->resource->appid);
 
     return 0;
 }
diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h
index 430d61fd3c..02fc16b17d 100644
--- a/src/conf/domain_validate.h
+++ b/src/conf/domain_validate.h
@@ -42,3 +42,5 @@ int virDomainDeviceDefValidate(const virDomainDeviceDef *dev,
 int virDomainDiskDefValidateSource(const virStorageSource *src);
 
 int virDomainDiskDefSourceLUNValidate(const virStorageSource *src);
+
+int virDomainDefResourceAppidValidate(const char *appid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2778fe7f8f..d1f5dc2080 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -765,6 +765,7 @@ virDomainConfVMNWFilterTeardown;
 
 # conf/domain_validate.h
 virDomainActualNetDefValidate;
+virDomainDefResourceAppidValidate;
 virDomainDefValidate;
 virDomainDeviceValidateAliasForHotplug;
 virDomainDiskDefSourceLUNValidate;
-- 
2.31.1

On 9/9/21 6:13 PM, Pavel Hrdina wrote:
> This will be needed by future patches adding appid API to allow changing
> it for running VMs.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/domain_validate.c | 42 +++++++++++++++++++++++---------------
>  src/conf/domain_validate.h |  2 ++
>  src/libvirt_private.syms   |  1 +
>  3 files changed, 28 insertions(+), 17 deletions(-)
> 
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 1bc62c364d..3ab864bbeb 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c


> +    if (def->resource->appid)
> +        return virDomainDefResourceAppidValidate(def->resource->appid);
>  

I'd write this as:

  if (def->resource->appid &&
      virDomainDefResourceAppidValidate() < 0)
      return -1;

so that this line doesn't have to be changed when something new is added
to this function. Moreover, you can have
virDomainDefResourceAppidValidate() to be NOP if appid == NULL and write
this check as:

  if (virDomainDefResourceAppidValidate() < 0)
    return -1;

Michal

On Fri, Sep 10, 2021 at 01:49:24PM +0200, Michal Prívozník wrote:
> On 9/9/21 6:13 PM, Pavel Hrdina wrote:
> > This will be needed by future patches adding appid API to allow changing
> > it for running VMs.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/conf/domain_validate.c | 42 +++++++++++++++++++++++---------------
> >  src/conf/domain_validate.h |  2 ++
> >  src/libvirt_private.syms   |  1 +
> >  3 files changed, 28 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> > index 1bc62c364d..3ab864bbeb 100644
> > --- a/src/conf/domain_validate.c
> > +++ b/src/conf/domain_validate.c
> 
> 
> > +    if (def->resource->appid)
> > +        return virDomainDefResourceAppidValidate(def->resource->appid);
> >  
> 
> I'd write this as:
> 
>   if (def->resource->appid &&
>       virDomainDefResourceAppidValidate() < 0)
>       return -1;
> 
> so that this line doesn't have to be changed when something new is added
> to this function. Moreover, you can have
> virDomainDefResourceAppidValidate() to be NOP if appid == NULL and write
> this check as:
> 
>   if (virDomainDefResourceAppidValidate() < 0)
>     return -1;

True, I'll fix that. Thanks.

Pavel