[libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c

Daniel Henrique Barboza posted 26 patches 6 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
Posted by Daniel Henrique Barboza 6 years, 2 months ago
Move smartcard validation being done by qemuBuildSmartcardCommandLine()
to the existing qemuDomainSmartcardDefValidate() function. This
function is called by qemuDomainDeviceDefValidate(), allowing smartcard
validation in domain define time.

Tests were adapted to consider the new caps being needed in
this earlier stage.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_command.c | 24 ------------------------
 src/qemu/qemu_domain.c  | 37 +++++++++++++++++++++++++++++++++++++
 tests/qemuxml2xmltest.c | 16 +++++++++-------
 3 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index efc70d6de9..116ed45a12 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8271,24 +8271,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
 
     switch (smartcard->type) {
     case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
-        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("this QEMU binary lacks smartcard host "
-                             "mode support"));
-            return -1;
-        }
-
         virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated");
         break;
 
     case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
-        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("this QEMU binary lacks smartcard host "
-                             "mode support"));
-            return -1;
-        }
-
         virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
         for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
             virBufferAsprintf(&opt, ",cert%zu=", i + 1);
@@ -8304,13 +8290,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
         break;
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("this QEMU binary lacks smartcard "
-                             "passthrough mode support"));
-            return -1;
-        }
-
         if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
                                               cmd, cfg, def,
                                               smartcard->data.passthru,
@@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
         break;
 
     default:
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unexpected smartcard type %d"),
-                       smartcard->type);
         return -1;
     }
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 415f0916a2..e4b70174ab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5956,6 +5956,43 @@ static int
 qemuDomainSmartcardDefValidate(const virDomainSmartcardDef *def,
                                virQEMUCapsPtr qemuCaps)
 {
+
+
+    switch (def->type) {
+    case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("this QEMU binary lacks smartcard host "
+                             "mode support"));
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("this QEMU binary lacks smartcard host "
+                             "mode support"));
+            return -1;
+        }
+        break;
+
+    case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("this QEMU binary lacks smartcard "
+                             "passthrough mode support"));
+            return -1;
+        }
+        break;
+
+    default:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected smartcard type %d"),
+                       def->type);
+        return -1;
+    }
+
     if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH &&
         qemuDomainChrSourceDefValidate(def->data.passthru, qemuCaps) < 0)
         return -1;
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b449c21f18..5c54060723 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1333,12 +1333,13 @@ mymain(void)
     DO_TEST("cpu-check-default-partial2", NONE);
     DO_TEST("vmcoreinfo", NONE);
 
-    DO_TEST("smartcard-host", NONE);
-    DO_TEST("smartcard-host-certificates", NONE);
-    DO_TEST("smartcard-host-certificates-database", NONE);
-    DO_TEST("smartcard-passthrough-tcp", NONE);
-    DO_TEST("smartcard-passthrough-spicevmc", NONE);
-    DO_TEST("smartcard-controller", NONE);
+    DO_TEST("smartcard-host", QEMU_CAPS_CCID_EMULATED);
+    DO_TEST("smartcard-host-certificates", QEMU_CAPS_CCID_EMULATED);
+    DO_TEST("smartcard-host-certificates-database",
+            QEMU_CAPS_CCID_EMULATED);
+    DO_TEST("smartcard-passthrough-tcp", QEMU_CAPS_CCID_PASSTHRU);
+    DO_TEST("smartcard-passthrough-spicevmc", QEMU_CAPS_CCID_PASSTHRU);
+    DO_TEST("smartcard-controller", QEMU_CAPS_CCID_EMULATED);
 
     DO_TEST("pseries-cpu-compat-power9",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
@@ -1353,7 +1354,8 @@ mymain(void)
             QEMU_CAPS_OBJECT_MEMORY_FILE,
             QEMU_CAPS_PIIX_DISABLE_S3,
             QEMU_CAPS_PIIX_DISABLE_S4,
-            QEMU_CAPS_VNC);
+            QEMU_CAPS_VNC,
+            QEMU_CAPS_CCID_EMULATED);
     DO_TEST("input-virtio-ccw",
             QEMU_CAPS_CCW,
             QEMU_CAPS_VIRTIO_KEYBOARD,
-- 
2.23.0


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

Re: [libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
Posted by Cole Robinson 6 years, 1 month ago
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
> Move smartcard validation being done by qemuBuildSmartcardCommandLine()
> to the existing qemuDomainSmartcardDefValidate() function. This
> function is called by qemuDomainDeviceDefValidate(), allowing smartcard
> validation in domain define time.
> 
> Tests were adapted to consider the new caps being needed in
> this earlier stage.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  src/qemu/qemu_command.c | 24 ------------------------
>  src/qemu/qemu_domain.c  | 37 +++++++++++++++++++++++++++++++++++++
>  tests/qemuxml2xmltest.c | 16 +++++++++-------
>  3 files changed, 46 insertions(+), 31 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index efc70d6de9..116ed45a12 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8271,24 +8271,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>  
>      switch (smartcard->type) {
>      case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("this QEMU binary lacks smartcard host "
> -                             "mode support"));
> -            return -1;
> -        }
> -
>          virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated");
>          break;
>  
>      case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("this QEMU binary lacks smartcard host "
> -                             "mode support"));
> -            return -1;
> -        }
> -
>          virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
>          for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
>              virBufferAsprintf(&opt, ",cert%zu=", i + 1);
> @@ -8304,13 +8290,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>          break;
>  
>      case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("this QEMU binary lacks smartcard "
> -                             "passthrough mode support"));
> -            return -1;
> -        }
> -
>          if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
>                                                cmd, cfg, def,
>                                                smartcard->data.passthru,
> @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>          break;
>  
>      default:
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("unexpected smartcard type %d"),
> -                       smartcard->type);
>          return -1;

This still returns -1. If somehow we hit this condition, the startup
will fail but no error will be reported. I think just 'return 0;' here
instead

- Cole

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

Re: [libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
Posted by Cole Robinson 6 years, 1 month ago
On 12/16/19 5:46 PM, Cole Robinson wrote:
> On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
>> Move smartcard validation being done by qemuBuildSmartcardCommandLine()
>> to the existing qemuDomainSmartcardDefValidate() function. This
>> function is called by qemuDomainDeviceDefValidate(), allowing smartcard
>> validation in domain define time.
>>
>> Tests were adapted to consider the new caps being needed in
>> this earlier stage.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>  src/qemu/qemu_command.c | 24 ------------------------
>>  src/qemu/qemu_domain.c  | 37 +++++++++++++++++++++++++++++++++++++
>>  tests/qemuxml2xmltest.c | 16 +++++++++-------
>>  3 files changed, 46 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index efc70d6de9..116ed45a12 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -8271,24 +8271,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>>  
>>      switch (smartcard->type) {
>>      case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
>> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
>> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -                           _("this QEMU binary lacks smartcard host "
>> -                             "mode support"));
>> -            return -1;
>> -        }
>> -
>>          virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated");
>>          break;
>>  
>>      case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
>> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
>> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -                           _("this QEMU binary lacks smartcard host "
>> -                             "mode support"));
>> -            return -1;
>> -        }
>> -
>>          virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
>>          for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
>>              virBufferAsprintf(&opt, ",cert%zu=", i + 1);
>> @@ -8304,13 +8290,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>>          break;
>>  
>>      case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
>> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
>> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -                           _("this QEMU binary lacks smartcard "
>> -                             "passthrough mode support"));
>> -            return -1;
>> -        }
>> -
>>          if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
>>                                                cmd, cfg, def,
>>                                                smartcard->data.passthru,
>> @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>>          break;
>>  
>>      default:
>> -        virReportError(VIR_ERR_INTERNAL_ERROR,
>> -                       _("unexpected smartcard type %d"),
>> -                       smartcard->type);
>>          return -1;
> 
> This still returns -1. If somehow we hit this condition, the startup
> will fail but no error will be reported. I think just 'return 0;' here
> instead

Actually virReportEnumRangeError seems like the common pattern in this case

- Cole

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