[v1] qemu: Watchdog plug/unplug

[libvirt] [PATCH 2/4] qemu: cold-unplug of watchdog

Posted by Michal Privoznik 8 years, 2 months ago

https://bugzilla.redhat.com/show_bug.cgi?id=1447169

Again, no special here.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f30a04b14..247d1175b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -539,6 +539,7 @@ virDomainVirtTypeFromString;
 virDomainVirtTypeToString;
 virDomainWatchdogActionTypeFromString;
 virDomainWatchdogActionTypeToString;
+virDomainWatchdogDefFree;
 virDomainWatchdogModelTypeFromString;
 virDomainWatchdogModelTypeToString;
 virDomainXMLOptionGetNamespace;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 583908972..85b4be360 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8155,10 +8155,19 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
         break;
 
 
+    case VIR_DOMAIN_DEVICE_WATCHDOG:
+        if (!vmdef->watchdog) {
+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                           _("domain has no watchdog"));
+            return -1;
+        }
+        virDomainWatchdogDefFree(vmdef->watchdog);
+        vmdef->watchdog = NULL;
+        break;
+
     case VIR_DOMAIN_DEVICE_INPUT:
     case VIR_DOMAIN_DEVICE_SOUND:
     case VIR_DOMAIN_DEVICE_VIDEO:
-    case VIR_DOMAIN_DEVICE_WATCHDOG:
     case VIR_DOMAIN_DEVICE_GRAPHICS:
     case VIR_DOMAIN_DEVICE_HUB:
     case VIR_DOMAIN_DEVICE_SMARTCARD:
-- 
2.13.5

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

Re: [libvirt] [PATCH 2/4] qemu: cold-unplug of watchdog

Posted by John Ferlan 8 years, 1 month ago


On 09/05/2017 07:45 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1447169
> 
> Again, no special here.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_driver.c   | 11 ++++++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 

Is there a specific reason to separate this from patch 1. I would think
attach and detach would be something added at the same time. OTOH, IDC
since the end result is still achieved.

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

John

BTW: Whether an error is really necessary could be debatable especially
since there can only be one... Still since other types error when not
found, I'm not opposed - just noting.

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f30a04b14..247d1175b 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -539,6 +539,7 @@ virDomainVirtTypeFromString;
>  virDomainVirtTypeToString;
>  virDomainWatchdogActionTypeFromString;
>  virDomainWatchdogActionTypeToString;
> +virDomainWatchdogDefFree;
>  virDomainWatchdogModelTypeFromString;
>  virDomainWatchdogModelTypeToString;
>  virDomainXMLOptionGetNamespace;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 583908972..85b4be360 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8155,10 +8155,19 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
>          break;
>  
>  
> +    case VIR_DOMAIN_DEVICE_WATCHDOG:
> +        if (!vmdef->watchdog) {
> +            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                           _("domain has no watchdog"));
> +            return -1;
> +        }
> +        virDomainWatchdogDefFree(vmdef->watchdog);
> +        vmdef->watchdog = NULL;
> +        break;
> +


>      case VIR_DOMAIN_DEVICE_INPUT:
>      case VIR_DOMAIN_DEVICE_SOUND:
>      case VIR_DOMAIN_DEVICE_VIDEO:
> -    case VIR_DOMAIN_DEVICE_WATCHDOG:
>      case VIR_DOMAIN_DEVICE_GRAPHICS:
>      case VIR_DOMAIN_DEVICE_HUB:
>      case VIR_DOMAIN_DEVICE_SMARTCARD:
> 

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

Re: [libvirt] [PATCH 2/4] qemu: cold-unplug of watchdog

Posted by Michal Privoznik 8 years, 1 month ago

On 09/12/2017 03:31 PM, John Ferlan wrote:
> 
> 
> On 09/05/2017 07:45 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>>
>> Again, no special here.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/qemu/qemu_driver.c   | 11 ++++++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
> 
> Is there a specific reason to separate this from patch 1. I would think
> attach and detach would be something added at the same time. OTOH, IDC
> since the end result is still achieved.

Yeah, they can be merged, they can be separated. Since I'm separating
live attach & detach (due to their complexity) I've separated these two
too (for consistency).

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

Pushed. Thanks.

Michal

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