[PATCH 0/3] ch: Misc fixes
*** BLURB HERE *** Michal Prívozník (3): ch: Use CH_DOMAIN_PRIVATE() more ch: Drop pid from monitor ch: Fix printf format strings wrt size_t argument src/ch/ch_events.c | 4 ++-- src/ch/ch_monitor.c | 14 ++++---------- src/ch/ch_monitor.h | 2 -- src/ch/ch_process.c | 11 +++++------ 4 files changed, 11 insertions(+), 20 deletions(-) -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> There are two instances where vm->privateData is typecasted only so that it can be dereferenced further. Well, that's exactly what CH_DOMAIN_PRIVATE() macro is for. Use that instead. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/ch/ch_events.c | 2 +- src/ch/ch_process.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index f1dc5c6f4c..9cb873d8cd 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -54,7 +54,7 @@ static int virCHEventStopProcess(virDomainObj *vm, virDomainShutoffReason reason) { - virCHDriver *driver = ((virCHDomainObjPrivate *)vm->privateData)->driver; + virCHDriver *driver = CH_DOMAIN_PRIVATE(vm)->driver; virObjectLock(vm); if (virDomainObjBeginJob(vm, VIR_JOB_DESTROY)) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index e8d482da97..6a59bf756a 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -430,8 +430,8 @@ virCHProcessSetupVcpus(virDomainObj *vm) size_t i; if ((vm->def->cputune.period || vm->def->cputune.quota) && - !virCgroupHasController(((virCHDomainObjPrivate *) vm->privateData)-> - cgroup, VIR_CGROUP_CONTROLLER_CPU)) { + !virCgroupHasController(CH_DOMAIN_PRIVATE(vm)->cgroup, + VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cgroup cpu is required for scheduler tuning")); return -1; -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> The domain object already has a member that allows storing hypervisor's PID (vm->pid). There's no need to duplicate it in _virCHMonitor struct. Switch CH code to use the former. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/ch/ch_events.c | 2 +- src/ch/ch_monitor.c | 10 ++-------- src/ch/ch_monitor.h | 2 -- src/ch/ch_process.c | 3 +-- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index 9cb873d8cd..3d4e3c41e1 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -297,7 +297,7 @@ int virCHStartEventHandler(virCHMonitor *mon) { g_autofree char *name = NULL; - name = g_strdup_printf("ch-evt-%d", mon->pid); + name = g_strdup_printf("ch-evt-%d", mon->vm->pid); virObjectRef(mon); if (virThreadCreateFull(&mon->event_handler_thread, diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 91899e873b..1c2c1f2858 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -672,14 +672,14 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile) return NULL; } - if ((rv = virPidFileReadPath(priv->pidfile, &mon->pid)) < 0) { + if ((rv = virPidFileReadPath(priv->pidfile, &vm->pid)) < 0) { virReportSystemError(-rv, _("Domain %1$s didn't show up"), vm->def->name); return NULL; } VIR_DEBUG("CH vm=%p name=%s running with pid=%lld", - vm, vm->def->name, (long long)mon->pid); + vm, vm->def->name, (long long)vm->pid); /* open the reader end of fifo before start Event Handler */ while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) { @@ -727,12 +727,6 @@ void virCHMonitorClose(virCHMonitor *mon) if (!mon) return; - if (mon->pid > 0) { - /* try cleaning up the Cloud-Hypervisor process */ - virProcessAbort(mon->pid); - mon->pid = 0; - } - if (mon->handle) curl_easy_cleanup(mon->handle); diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index 185de0dbfd..ffac9e938e 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -108,8 +108,6 @@ struct _virCHMonitor { size_t buf_fill_sz; } event_buffer; - pid_t pid; - virDomainObj *vm; size_t nthreads; diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 6a59bf756a..a008b52752 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -957,7 +957,6 @@ virCHProcessStart(virCHDriver *driver, } } - vm->pid = priv->monitor->pid; vm->def->id = vm->pid; priv->machineName = virCHDomainGetMachineName(vm); @@ -1024,6 +1023,7 @@ virCHProcessStop(virCHDriver *driver, virErrorPreserveLast(&orig_err); if (priv->monitor) { + virProcessAbort(vm->pid); g_clear_pointer(&priv->monitor, virCHMonitorClose); } @@ -1117,7 +1117,6 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from } } - vm->pid = priv->monitor->pid; vm->def->id = vm->pid; priv->machineName = virCHDomainGetMachineName(vm); -- 2.49.0
From: Michal Privoznik <mprivozn@redhat.com> In a few places, when a size_t typed argument is passed to a printf-like function the corresponding specifier is %ld instead of %zu. Fix those places. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/ch/ch_monitor.c | 4 ++-- src/ch/ch_process.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 1c2c1f2858..78b6551d6c 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -156,7 +156,7 @@ virCHMonitorBuildPayloadJson(virJSONValue *content, virDomainDef *vmdef) buf = g_base64_decode(vmdef->sec->data.sev_snp.host_data, &len); if (len != host_data_len) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid host_data provided. Expected '%1$ld' bytes"), + _("Invalid host_data provided. Expected '%1$zu' bytes"), host_data_len); return -1; } @@ -1139,7 +1139,7 @@ virCHMonitorBuildRestoreJson(virDomainDef *vmdef, g_autoptr(virJSONValue) nets = virJSONValueNewArray(); for (i = 0; i < vmdef->nnets; i++) { g_autoptr(virJSONValue) net_json = virJSONValueNewObject(); - g_autofree char *id = g_strdup_printf("%s_%ld", CH_NET_ID_PREFIX, i); + g_autofree char *id = g_strdup_printf("%s_%zu", CH_NET_ID_PREFIX, i); if (virJSONValueObjectAppendString(net_json, "id", id) < 0) return -1; if (virJSONValueObjectAppendNumberInt(net_json, "num_fds", vmdef->nets[i]->driver.virtio.queues)) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index a008b52752..95c808cb41 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -701,7 +701,7 @@ chProcessAddNetworkDevices(virCHDriver *driver, VIR_DEBUG("payload sent with net-add request to CH = %s", payload); virBufferAsprintf(&buf, "%s", virBufferCurrentContent(&http_headers)); - virBufferAsprintf(&buf, "Content-Length: %ld\r\n\r\n", strlen(payload)); + virBufferAsprintf(&buf, "Content-Length: %zu\r\n\r\n", strlen(payload)); virBufferAsprintf(&buf, "%s", payload); payload_len = virBufferUse(&buf); payload = virBufferContentAndReset(&buf); @@ -1130,7 +1130,7 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from virBufferAddLit(&http_headers, "Host: localhost\r\n"); virBufferAddLit(&http_headers, "Content-Type: application/json\r\n"); virBufferAsprintf(&buf, "%s", virBufferCurrentContent(&http_headers)); - virBufferAsprintf(&buf, "Content-Length: %ld\r\n\r\n", strlen(payload)); + virBufferAsprintf(&buf, "Content-Length: %zu\r\n\r\n", strlen(payload)); virBufferAsprintf(&buf, "%s", payload); payload_len = virBufferUse(&buf); payload = virBufferContentAndReset(&buf); -- 2.49.0
On a Thursday in 2025, Michal Privoznik via Devel wrote:
*** BLURB HERE ***
Michal Prívozník (3): ch: Use CH_DOMAIN_PRIVATE() more ch: Drop pid from monitor ch: Fix printf format strings wrt size_t argument
src/ch/ch_events.c | 4 ++-- src/ch/ch_monitor.c | 14 ++++---------- src/ch/ch_monitor.h | 2 -- src/ch/ch_process.c | 11 +++++------ 4 files changed, 11 insertions(+), 20 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik