2:33 a.m.
v2 of:
https://www.redhat.com/archives/libvir-list/2017-September/msg00078.html
diff to v1:
- John's review worked in (mostly).
- Added auditing (patch 1/3)
Michal Privoznik (3):
audit: Audit information about watchdog devices
qemu: hot-plug of watchdog
qemu: hot-unplug of watchdog
src/conf/domain_audit.c | 46 ++++++++
src/conf/domain_audit.h | 5 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_alias.c | 13 ++-
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 4 +-
src/qemu/qemu_driver.c | 14 ++-
src/qemu/qemu_hotplug.c | 129 +++++++++++++++++++++
src/qemu/qemu_hotplug.h | 6 +
src/qemu/qemu_monitor.c | 12 ++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 27 +++++
src/qemu/qemu_monitor_json.h | 3 +
tests/qemuhotplugtest.c | 14 ++-
.../qemuhotplug-watchdog-full.xml | 3 +
.../qemuhotplug-watchdog.xml | 1 +
.../qemuhotplug-base-live+watchdog.xml | 55 +++++++++
18 files changed, 333 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
--
2.13.5
2:33 a.m.
New subject: [libvirt] [PATCH v2 1/3] audit: Audit information about watchdog devices
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_audit.h | 5 +++++
src/libvirt_private.syms | 1 +
3 files changed, 52 insertions(+)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 4afc22019..026fafe0c 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -865,6 +865,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
if (vm->def->tpm)
virDomainAuditTPM(vm, vm->def->tpm, "start", true);
+ if (vm->def->watchdog)
+ virDomainAuditWatchdog(vm, vm->def->watchdog, "start", true);
+
for (i = 0; i < vm->def->nshmems; i++)
virDomainAuditShmem(vm, vm->def->shmems[i], "start", true);
@@ -983,3 +986,46 @@ virDomainAuditShmem(virDomainObjPtr vm,
VIR_FREE(shmpath);
return;
}
+
+
+void
+virDomainAuditWatchdog(virDomainObjPtr vm,
+ virDomainWatchdogDefPtr def,
+ const char *reason, bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char *vmname;
+ char *alias = NULL;
+ char *device = NULL;
+ const char *virt;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+ if (!(vmname = virAuditEncode("vm", vm->def->name))) {
+ VIR_WARN("OOM while encoding audit message");
+ return;
+ }
+
+ if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
+ VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
+ virt = "?";
+ }
+
+ if (VIR_STRDUP_QUIET(alias, def->info.alias) < 0) {
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ if (!(device = virAuditEncode("device", VIR_AUDIT_STR(alias)))) {
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "virt=%s resrc=dev reason=%s %s uuid=%s %s",
+ virt, reason, vmname, uuidstr, device);
+
+ cleanup:
+ VIR_FREE(vmname);
+ VIR_FREE(device);
+ VIR_FREE(alias);
+}
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h
index 8cb585dc7..3a0e0bd39 100644
--- a/src/conf/domain_audit.h
+++ b/src/conf/domain_audit.h
@@ -134,5 +134,10 @@ void virDomainAuditShmem(virDomainObjPtr vm,
const char *reason, bool success)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void virDomainAuditWatchdog(virDomainObjPtr vm,
+ virDomainWatchdogDefPtr def,
+ const char *reason, bool success)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
#endif /* __VIR_DOMAIN_AUDIT_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5b1bc5e4f..68ff1f2c3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -167,6 +167,7 @@ virDomainAuditShmem;
virDomainAuditStart;
virDomainAuditStop;
virDomainAuditVcpu;
+virDomainAuditWatchdog;
# conf/domain_capabilities.h
--
2.13.5
3:41 a.m.
New subject: [libvirt] [PATCH v2 1/3] audit: Audit information about watchdog devices
On Wed, Sep 27, 2017 at 09:33:15 +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_audit.h | 5 +++++
src/libvirt_private.syms | 1 +
3 files changed, 52 insertions(+)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 4afc22019..026fafe0c 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
[...]
@@ -983,3 +986,46 @@ virDomainAuditShmem(virDomainObjPtr vm,
VIR_FREE(shmpath);
return;
}
+
+
+void
+virDomainAuditWatchdog(virDomainObjPtr vm,
+ virDomainWatchdogDefPtr def,
+ const char *reason, bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char *vmname;
+ char *alias = NULL;
+ char *device = NULL;
+ const char *virt;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+ if (!(vmname = virAuditEncode("vm", vm->def->name))) {
+ VIR_WARN("OOM while encoding audit message");
+ return;
+ }
+
+ if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
+ VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
+ virt = "?";
+ }
+
+ if (VIR_STRDUP_QUIET(alias, def->info.alias) < 0) {
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ if (!(device = virAuditEncode("device", VIR_AUDIT_STR(alias)))) {
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "virt=%s resrc=dev reason=%s %s uuid=%s %s",
+ virt, reason, vmname, uuidstr, device);
The audit message should identify the host resource which is used by the
VM. I doubt that the alias is a good reference to the host device in use.
4:10 a.m.
New subject: [libvirt] [PATCH v2 1/3] audit: Audit information about watchdog devices
On 09/27/2017 10:41 AM, Peter Krempa wrote:
On Wed, Sep 27, 2017 at 09:33:15 +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_audit.h | 5 +++++
> src/libvirt_private.syms | 1 +
> 3 files changed, 52 insertions(+)
>
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index 4afc22019..026fafe0c 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
[...]
> @@ -983,3 +986,46 @@ virDomainAuditShmem(virDomainObjPtr vm,
> VIR_FREE(shmpath);
> return;
> }
> +
> +
> +void
> +virDomainAuditWatchdog(virDomainObjPtr vm,
> + virDomainWatchdogDefPtr def,
> + const char *reason, bool success)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + char *vmname;
> + char *alias = NULL;
> + char *device = NULL;
> + const char *virt;
> +
> + virUUIDFormat(vm->def->uuid, uuidstr);
> + if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> + VIR_WARN("OOM while encoding audit message");
> + return;
> + }
> +
> + if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
> + VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
> + virt = "?";
> + }
> +
> + if (VIR_STRDUP_QUIET(alias, def->info.alias) < 0) {
> + VIR_WARN("OOM while encoding audit message");
> + goto cleanup;
> + }
> +
> + if (!(device = virAuditEncode("device", VIR_AUDIT_STR(alias)))) {
> + VIR_WARN("OOM while encoding audit message");
> + goto cleanup;
> + }
> +
> + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
> + "virt=%s resrc=dev reason=%s %s uuid=%s %s",
> + virt, reason, vmname, uuidstr, device);
The audit message should identify the host resource which is used by the
VM. I doubt that the alias is a good reference to the host device in use.
There's no host resource to report. Watchdogs are purely virtual.
Therefore I went with the alias. But feel free to suggest a better option.
Michal
4:17 a.m.
New subject: [libvirt] [PATCH v2 1/3] audit: Audit information about watchdog devices
On Wed, Sep 27, 2017 at 11:10:31 +0200, Michal Privoznik wrote:
On 09/27/2017 10:41 AM, Peter Krempa wrote:
> On Wed, Sep 27, 2017 at 09:33:15 +0200, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/conf/domain_audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_audit.h | 5 +++++
>> src/libvirt_private.syms | 1 +
>> 3 files changed, 52 insertions(+)
>>
>> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
>> index 4afc22019..026fafe0c 100644
>> --- a/src/conf/domain_audit.c
>> +++ b/src/conf/domain_audit.c
[...]
>
>> + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
>> + "virt=%s resrc=dev reason=%s %s uuid=%s %s",
>> + virt, reason, vmname, uuidstr, device);
>
> The audit message should identify the host resource which is used by the
> VM. I doubt that the alias is a good reference to the host device in use.
>
>
There's no host resource to report. Watchdogs are purely virtual.
Therefore I went with the alias. But feel free to suggest a better option.
Ah, in that case NACK. We don't audit purely virtual resources.
2:33 a.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: hot-plug of watchdog
https://bugzilla.redhat.com/show_bug.cgi?id=1447169
Once again, since domain can have at most one watchdog it
simplifies things a bit. However, since we must be able to set
the watchdog action as well, new monitor command needs to be
used.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_alias.c | 13 ++++-
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 4 +-
src/qemu/qemu_driver.c | 10 +++-
src/qemu/qemu_hotplug.c | 68 ++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 +
src/qemu/qemu_monitor.c | 12 ++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 27 +++++++++
src/qemu/qemu_monitor_json.h | 3 +
tests/qemuhotplugtest.c | 9 ++-
.../qemuhotplug-watchdog.xml | 1 +
.../qemuhotplug-base-live+watchdog.xml | 55 +++++++++++++++++
14 files changed, 206 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 914b2b94d..72df1083f 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -405,6 +405,17 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def,
}
+int
+qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog)
+{
+ /* Currently, there's just one watchdog per domain */
+
+ if (VIR_STRDUP(watchdog->info.alias, "watchdog0") < 0)
+ return -1;
+ return 0;
+}
+
+
int
qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
{
@@ -482,7 +493,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
if (def->watchdog) {
- if (virAsprintf(&def->watchdog->info.alias, "watchdog%d", 0)
< 0)
+ if (qemuAssignDeviceWatchdogAlias(def->watchdog) < 0)
return -1;
}
if (def->memballoon) {
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 300fd4de5..652ffea0c 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -65,6 +65,8 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def,
virDomainShmemDefPtr shmem,
int idx);
+int qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog);
+
int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index abeb24846..5ded0ae79 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3964,7 +3964,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
-static char *
+char *
qemuBuildWatchdogDevStr(const virDomainDef *def,
virDomainWatchdogDefPtr dev,
virQEMUCapsPtr qemuCaps)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 6fbfb3e5f..94e4592cc 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -205,6 +205,8 @@ char *qemuBuildShmemDevStr(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
-
+char *qemuBuildWatchdogDevStr(const virDomainDef *def,
+ virDomainWatchdogDefPtr dev,
+ virQEMUCapsPtr qemuCaps);
#endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4855c9047..db8ad0b04 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7648,12 +7648,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
}
break;
+ case VIR_DOMAIN_DEVICE_WATCHDOG:
+ ret = qemuDomainAttachWatchdog(driver, vm,
+ dev->data.watchdog);
+ if (!ret) {
+ alias = dev->data.watchdog->info.alias;
+ dev->data.watchdog = NULL;
+ }
+ break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
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:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4913e18e6..885483c0f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2836,6 +2836,74 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
}
+int
+qemuDomainAttachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr watchdog)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_WATCHDOG, { .watchdog = watchdog } };
+ virDomainWatchdogAction actualAction = watchdog->action;
+ const char *actionStr = NULL;
+ char *watchdogstr = NULL;
+ bool releaseAddress = false;
+ int rv;
+
+ if (vm->def->watchdog) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain already has a watchdog"));
+ return -1;
+ }
+
+ if (qemuAssignDeviceWatchdogAlias(watchdog) < 0)
+ return -1;
+
+ if (!(watchdogstr = qemuBuildWatchdogDevStr(vm->def, watchdog,
priv->qemuCaps)))
+ return -1;
+
+ if (watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ goto cleanup;
+ releaseAddress = true;
+ }
+
+ /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause',
then
+ libvirt listens for the watchdog event, and we perform the dump
+ ourselves. so convert 'dump' to 'pause' for the qemu cli */
+ if (actualAction == VIR_DOMAIN_WATCHDOG_ACTION_DUMP)
+ actualAction = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
+
+ actionStr = virDomainWatchdogActionTypeToString(actualAction);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ rv = qemuMonitorSetWatchdogAction(priv->mon, actionStr);
+
+ if (rv >= 0)
+ rv = qemuMonitorAddDevice(priv->mon, watchdogstr);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ releaseAddress = false;
+ virDomainAuditWatchdog(vm, watchdog, "attach", rv >= 0);
+ goto cleanup;
+ }
+
+ if (rv < 0)
+ goto cleanup;
+
+ releaseAddress = false;
+ vm->def->watchdog = watchdog;
+ ret = 0;
+
+ cleanup:
+ if (releaseAddress)
+ qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
+ VIR_FREE(watchdogstr);
+ return ret;
+}
+
+
static int
qemuDomainChangeNetBridge(virDomainObjPtr vm,
virDomainNetDefPtr olddev,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 985c6733f..a9dfd8f7a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -80,6 +80,9 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainShmemDefPtr shmem);
+int qemuDomainAttachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr watchdog);
int qemuDomainFindGraphicsIndex(virDomainDefPtr def,
virDomainGraphicsDefPtr dev);
int qemuDomainAttachMemory(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 363ad76cf..7a2678587 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4339,3 +4339,15 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info)
VIR_FREE(info);
}
+
+
+int
+qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
+ const char *action)
+{
+ VIR_DEBUG("watchdogAction=%s", action);
+
+ QEMU_CHECK_MONITOR_JSON(mon);
+
+ return qemuMonitorJSONSetWatchdogAction(mon, action);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6414d2483..d9c27acae 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1129,4 +1129,6 @@ int qemuMonitorSetBlockThreshold(qemuMonitorPtr mon,
virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon);
+int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
+ const char *action);
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 63b855920..9876939ca 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7692,3 +7692,30 @@ qemuMonitorJSONQueryNamedBlockNodes(qemuMonitorPtr mon)
return ret;
}
+
+
+int
+qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon,
+ const char *action)
+{
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("watchdog-set-action",
+ "s:action", action,
+ NULL)))
+ return -1;
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 0cdfc5ead..f418c7426 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -521,4 +521,7 @@ int qemuMonitorJSONSetBlockThreshold(qemuMonitorPtr mon,
virJSONValuePtr qemuMonitorJSONQueryNamedBlockNodes(qemuMonitorPtr mon)
ATTRIBUTE_NONNULL(1);
+int qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon,
+ const char *action)
+ ATTRIBUTE_NONNULL(1);
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 23a498617..b02ae8034 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -127,6 +127,9 @@ testQemuHotplugAttach(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SHMEM:
ret = qemuDomainAttachShmemDevice(&driver, vm, dev->data.shmem);
break;
+ case VIR_DOMAIN_DEVICE_WATCHDOG:
+ ret = qemuDomainAttachWatchdog(&driver, vm, dev->data.watchdog);
+ break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be attached\n",
virDomainDeviceTypeToString(dev->type));
@@ -811,10 +814,14 @@ mymain(void)
"device_del", QMP_OK,
"object-del", QMP_OK);
DO_TEST_ATTACH("base-live+disk-scsi-wwn",
- "disk-scsi-duplicate-wwn", false, true,
+ "disk-scsi-duplicate-wwn", false, false,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
+ DO_TEST_ATTACH("base-live", "watchdog", false, false,
+ "watchdog-set-action", QMP_OK,
+ "device_add", QMP_OK);
+
#define DO_TEST_CPU_GROUP(prefix, vcpus, modernhp, expectfail) \
do { \
cpudata.test = prefix; \
diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
new file mode 100644
index 000000000..7e0e0a863
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
@@ -0,0 +1 @@
+<watchdog model='ib700' action='poweroff'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
new file mode 100644
index 000000000..f884eec50
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
@@ -0,0 +1,55 @@
+<domain type='kvm' id='7'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0'>
+ <alias name='usb'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <alias name='ide'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <alias name='scsi0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'>
+ <alias name='pci'/>
+ </controller>
+ <controller type='virtio-serial' index='0'>
+ <alias name='virtio-serial0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <input type='mouse' bus='ps2'>
+ <alias name='input0'/>
+ </input>
+ <input type='keyboard' bus='ps2'>
+ <alias name='input1'/>
+ </input>
+ <watchdog model='ib700' action='poweroff'>
+ <alias name='watchdog0'/>
+ </watchdog>
+ <memballoon model='none'>
+ <alias name='balloon0'/>
+ </memballoon>
+ </devices>
+ <seclabel type='none' model='none'/>
+</domain>
--
2.13.5
4:07 a.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: hot-plug of watchdog
On Wed, Sep 27, 2017 at 09:33:16 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1447169
Once again, since domain can have at most one watchdog it
again? You are mentioning it the first time in this series.
simplifies things a bit. However, since we must be able to set
the watchdog action as well, new monitor command needs to be
used.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_alias.c | 13 ++++-
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 4 +-
src/qemu/qemu_driver.c | 10 +++-
src/qemu/qemu_hotplug.c | 68 ++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 +
src/qemu/qemu_monitor.c | 12 ++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 27 +++++++++
src/qemu/qemu_monitor_json.h | 3 +
tests/qemuhotplugtest.c | 9 ++-
.../qemuhotplug-watchdog.xml | 1 +
.../qemuhotplug-base-live+watchdog.xml | 55 +++++++++++++++++
14 files changed, 206 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog.xml
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4913e18e6..885483c0f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2836,6 +2836,74 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
}
+int
+qemuDomainAttachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr watchdog)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_WATCHDOG, { .watchdog = watchdog } };
+ virDomainWatchdogAction actualAction = watchdog->action;
+ const char *actionStr = NULL;
+ char *watchdogstr = NULL;
+ bool releaseAddress = false;
+ int rv;
+
+ if (vm->def->watchdog) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain already has a watchdog"));
+ return -1;
+ }
+
+ if (qemuAssignDeviceWatchdogAlias(watchdog) < 0)
+ return -1;
+
+ if (!(watchdogstr = qemuBuildWatchdogDevStr(vm->def, watchdog,
priv->qemuCaps)))
+ return -1;
+
+ if (watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
If you are not going to allow auto-generation of PCI address you should
explicitly reject ADDRESS_TYPE_NONE.
Otherwise the address won't be reserved. If the address type is based on
the model you'll need to be able to figure it out.
Also I presume that the ISA based ones won't support hotplug so you
could deal with that too.
+ if (qemuDomainEnsurePCIAddress(vm, &dev, driver) <
0)
+ goto cleanup;
+ releaseAddress = true;
+ }
+
+ /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause',
then
+ libvirt listens for the watchdog event, and we perform the dump
+ ourselves. so convert 'dump' to 'pause' for the qemu cli */
+ if (actualAction == VIR_DOMAIN_WATCHDOG_ACTION_DUMP)
+ actualAction = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
+
+ actionStr = virDomainWatchdogActionTypeToString(actualAction);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ rv = qemuMonitorSetWatchdogAction(priv->mon, actionStr);
+
+ if (rv >= 0)
+ rv = qemuMonitorAddDevice(priv->mon, watchdogstr);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ releaseAddress = false;
+ virDomainAuditWatchdog(vm, watchdog, "attach", rv >= 0);
Reporting success here (in case of rv >= 0) is bogus since the VM died.
+ goto cleanup;
+ }
+
+ if (rv < 0)
+ goto cleanup;
+
+ releaseAddress = false;
+ vm->def->watchdog = watchdog;
+ ret = 0;
+
+ cleanup:
+ if (releaseAddress)
+ qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
+ VIR_FREE(watchdogstr);
+ return ret;
+}
+
+
static int
qemuDomainChangeNetBridge(virDomainObjPtr vm,
virDomainNetDefPtr olddev,
[...]
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 63b855920..9876939ca 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7692,3 +7692,30 @@ qemuMonitorJSONQueryNamedBlockNodes(qemuMonitorPtr mon)
return ret;
}
+
+
+int
+qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon,
+ const char *action)
+{
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+ int ret = -1;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("watchdog-set-action",
+ "s:action", action,
+ NULL)))
+ return -1;
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
Inconsistent spacing.
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
2:33 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: hot-unplug of watchdog
https://bugzilla.redhat.com/show_bug.cgi?id=1447169
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 61 ++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 ++
tests/qemuhotplugtest.c | 7 ++-
.../qemuhotplug-watchdog-full.xml | 3 ++
5 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index db8ad0b04..4c8e9297a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7747,12 +7747,14 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SHMEM:
ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
break;
+ case VIR_DOMAIN_DEVICE_WATCHDOG:
+ ret = qemuDomainDetachWatchdog(driver, vm, dev->data.watchdog);
+ break;
case VIR_DOMAIN_DEVICE_FS:
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:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 885483c0f..477ad2cdd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4333,6 +4333,26 @@ qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainRemoveWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr watchdog)
+{
+ virObjectEventPtr event = NULL;
+
+ VIR_DEBUG("Removing watchdog %s from domain %p %s",
+ watchdog->info.alias, vm, vm->def->name);
+
+ virDomainAuditWatchdog(vm, watchdog, "detach", true);
+ event = virDomainEventDeviceRemovedNewFromObj(vm, watchdog->info.alias);
+ qemuDomainEventQueue(driver, event);
+ qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
+ virDomainWatchdogDefFree(vm->def->watchdog);
+ vm->def->watchdog = NULL;
+ return 0;
+}
+
+
int
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -5049,6 +5069,47 @@ qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
}
+int
+qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr dev)
+{
+ int ret = -1;
+ virDomainWatchdogDefPtr watchdog = vm->def->watchdog;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ /* While domains can have up to one watchdog, the one supplied by the user
+ * doesn't necessarily match the one domain has. Refuse to detach in such
+ * case. */
+ if (!(watchdog &&
+ STREQ_NULLABLE(dev->info.alias, watchdog->info.alias) &&
+ watchdog->model == dev->model &&
+ watchdog->action == dev->action)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("watchdog device not present in domain
configuration"));
+ return -1;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, &watchdog->info);
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ ret = qemuMonitorDelDevice(priv->mon, watchdog->info.alias);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ ret = -1;
+
+ if (ret == 0) {
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+ qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
+ ret = qemuDomainRemoveWatchdog(driver, vm, watchdog);
+ }
+ }
+ qemuDomainResetDeviceRemoval(vm);
+
+ return ret;
+}
+
+
int
qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index a9dfd8f7a..3455832d6 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -122,6 +122,9 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainShmemDefPtr dev);
+int qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr watchdog);
int qemuDomainAttachLease(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainLeaseDefPtr lease);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index b02ae8034..df28a7fbd 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -155,6 +155,9 @@ testQemuHotplugDetach(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SHMEM:
ret = qemuDomainDetachShmemDevice(&driver, vm, dev->data.shmem);
break;
+ case VIR_DOMAIN_DEVICE_WATCHDOG:
+ ret = qemuDomainDetachWatchdog(&driver, vm, dev->data.watchdog);
+ break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be detached\n",
virDomainDeviceTypeToString(dev->type));
@@ -818,9 +821,11 @@ mymain(void)
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_ATTACH("base-live", "watchdog", false, false,
+ DO_TEST_ATTACH("base-live", "watchdog", false, true,
"watchdog-set-action", QMP_OK,
"device_add", QMP_OK);
+ DO_TEST_DETACH("base-live", "watchdog-full", false, false,
+ "device_del", QMP_OK);
#define DO_TEST_CPU_GROUP(prefix, vcpus, modernhp, expectfail) \
do { \
diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
new file mode 100644
index 000000000..28827b58a
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
@@ -0,0 +1,3 @@
+<watchdog model='ib700' action='poweroff'>
+ <alias name='watchdog0'/>
+</watchdog>
--
2.13.5
4:15 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: hot-unplug of watchdog
On Wed, Sep 27, 2017 at 09:33:17 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1447169
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 61 ++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 ++
tests/qemuhotplugtest.c | 7 ++-
.../qemuhotplug-watchdog-full.xml | 3 ++
5 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog-full.xml
[...]
@@ -5049,6 +5069,47 @@ qemuDomainDetachShmemDevice(virQEMUDriverPtr
driver,
}
+int
+qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainWatchdogDefPtr dev)
+{
+ int ret = -1;
+ virDomainWatchdogDefPtr watchdog = vm->def->watchdog;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ /* While domains can have up to one watchdog, the one supplied by the user
+ * doesn't necessarily match the one domain has. Refuse to detach in such
+ * case. */
+ if (!(watchdog &&
+ STREQ_NULLABLE(dev->info.alias, watchdog->info.alias) &&
So this mandates that the alias is provided in the detach XML.
+ watchdog->model == dev->model &&
+ watchdog->action == dev->action)) {
But e.g. does not check whether the device info matches. For other
devices we don't mandate most fields. (especially the alias)
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("watchdog device not present in domain
configuration"));
+ return -1;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, &watchdog->info);
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ ret = qemuMonitorDelDevice(priv->mon, watchdog->info.alias);
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ ret = -1;
+
+ if (ret == 0) {
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+ qemuDomainReleaseDeviceAddress(vm, &watchdog->info, NULL);
+ ret = qemuDomainRemoveWatchdog(driver, vm, watchdog);
+ }
+ }
+ qemuDomainResetDeviceRemoval(vm);
+
+ return ret;
+}
+
+
int
qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
2616
days inactive
2616
days old
8 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa