[RFC 0/7] introduce support for live appid updates
I'm posting this as an RFC mainly because I'm not sure how to model the new API. This patches introduce a new naive API that will change only the APPID and nothing else. Currently there are no other known features related to Fibre Channel resources so this non-extendable API will be sufficient, however the appid lives in <resource> element in the XML where we currently have root cgroup partition. Even though changing the partition will not be supported and we don't know about anything else that could be placed here it doesn't mean it will not happen in the future. In that case we would have to add new API as well. So I'm wondering if we should create a more generic API that would take typed parameters as arguments: int virDomainSetResource(virDomainPtr domain, virTypedParameterPtr params, int nparams, unsigned int flags) Any ideas? Pavel Hrdina (7): conf: extract appid validation to virDomainDefResourceAppidValidate cgroup: extract setting fibre channel appid into virCgroupSetFCAppid virCgroupSetFCAppid: properly handle when appid is NULL src: introduce virDomainSetFibreChannelAppid API remote: add RPC support for the virDomainSetFibreChannelAppid API qemu: implement virDomainSetFibreChannelAppid API tools: introduce virsh setappid command docs/manpages/virsh.rst | 14 ++++++ include/libvirt/libvirt-domain.h | 4 ++ src/conf/domain_validate.c | 42 ++++++++++-------- src/conf/domain_validate.h | 2 + src/driver-hypervisor.h | 6 +++ src/libvirt-domain.c | 44 +++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 1 + src/qemu/qemu_cgroup.c | 17 +------- src/qemu/qemu_driver.c | 75 ++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++- src/remote_protocol-structs | 6 +++ src/util/vircgroup.c | 24 ++++++++++ src/util/vircgroup.h | 3 ++ tools/virsh-domain.c | 65 +++++++++++++++++++++++++++ 16 files changed, 286 insertions(+), 34 deletions(-) -- 2.31.1
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 60f7ccdddd..47e53f197d 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 38a1abcc8f..6013488ccf 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -40,3 +40,5 @@ int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, void *parseOpaque); int virDomainDiskDefValidateSource(const virStorageSource *src); + +int virDomainDefResourceAppidValidate(const char *appid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fa11ee3df5..e13dacf590 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -765,6 +765,7 @@ virDomainConfVMNWFilterTeardown; # conf/domain_validate.h virDomainActualNetDefValidate; +virDomainDefResourceAppidValidate; virDomainDefValidate; virDomainDeviceValidateAliasForHotplug; -- 2.31.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_cgroup.c | 17 +---------------- src/util/vircgroup.c | 24 ++++++++++++++++++++++++ src/util/vircgroup.h | 3 +++ 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e13dacf590..0051a5574c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1954,6 +1954,7 @@ virCgroupSetCpusetCpus; virCgroupSetCpusetMemoryMigrate; virCgroupSetCpusetMems; virCgroupSetCpuShares; +virCgroupSetFCAppid; virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 42dba1750d..736ef11c6e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -908,27 +908,12 @@ static int qemuSetupCgroupAppid(virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; - int inode = -1; - const char *path = "/sys/class/fc/fc_udev_device/appid_store"; - g_autofree char *appid = NULL; virDomainResourceDef *resource = vm->def->resource; if (!resource || !resource->appid) return 0; - inode = virCgroupGetInode(priv->cgroup); - if (inode < 0) - return -1; - - appid = g_strdup_printf("%X:%s", inode, resource->appid); - - if (virFileWriteStr(path, appid, 0) < 0) { - virReportSystemError(errno, - _("Unable to write '%s' to '%s'"), appid, path); - return -1; - } - - return 0; + return virCgroupSetFCAppid(priv->cgroup, resource->appid); } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 37b63a2e2d..ad0ee20862 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -4012,3 +4012,27 @@ virCgroupGetCpuPeriodQuota(virCgroup *cgroup, unsigned long long *period, return 0; } + + +int +virCgroupSetFCAppid(virCgroup *group, + const char *appid) +{ + int inode = -1; + const char *path = "/sys/class/fc/fc_udev_device/appid_store"; + g_autofree char *vmid = NULL; + + inode = virCgroupGetInode(group); + if (inode < 0) + return -1; + + vmid = g_strdup_printf("%X:%s", inode, appid); + + if (virFileWriteStr(path, vmid, 0) < 0) { + virReportSystemError(errno, + _("Unable to write '%s' to '%s'"), vmid, path); + return -1; + } + + return 0; +} diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 690f09465c..a69b435b71 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -285,3 +285,6 @@ int virCgroupHasEmptyTasks(virCgroup *cgroup, int controller); bool virCgroupControllerAvailable(int controller); int virCgroupGetInode(virCgroup *cgroup); + +int virCgroupSetFCAppid(virCgroup *group, + const char *appid); -- 2.31.1
With introduction of live changes of appid we should also support removal of the appid from VM. This is done by writing empty appid part to appid_store file. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index ad0ee20862..9470ab061d 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -4026,7 +4026,7 @@ virCgroupSetFCAppid(virCgroup *group, if (inode < 0) return -1; - vmid = g_strdup_printf("%X:%s", inode, appid); + vmid = g_strdup_printf("%X:%s", inode, appid ? appid : ""); if (virFileWriteStr(path, vmid, 0) < 0) { virReportSystemError(errno, -- 2.31.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 6 +++++ src/libvirt-domain.c | 44 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 55 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7ef8ac51e5..4f7b88ef61 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5153,4 +5153,8 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain, int seconds, unsigned int flags); +int virDomainSetFibreChannelAppid(virDomainPtr domain, + const char *appid, + unsigned int flags); + #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index d642af8a37..e6b1ceb3ce 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1412,6 +1412,11 @@ typedef int typedef struct _virHypervisorDriver virHypervisorDriver; +typedef int +(*virDrvDomainSetFibreChannelAppid)(virDomainPtr domain, + const char *appid, + unsigned int flags); + /** * _virHypervisorDriver: * @@ -1676,4 +1681,5 @@ struct _virHypervisorDriver { virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet; virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; + virDrvDomainSetFibreChannelAppid domainSetFibreChannelAppid; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a8a386e839..f3e6854a39 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13229,3 +13229,47 @@ virDomainStartDirtyRateCalc(virDomainPtr domain, virDispatchError(conn); return -1; } + + +/** + * virDomainSetFibreChannelAppid: + * @domain: a domain object + * @appid: user provided appid string + * @flags: extra flags + * + * Set the Fibre Channel APPID. Accepts only printable characters + * and maximal length is 128 characters. To remove the APPID use + * NULL as @appid value. + * + * Returns 0 in case of success, -1 otherwise. + */ +int +virDomainSetFibreChannelAppid(virDomainPtr domain, + const char *appid, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "appid=%s, flags=0x%x", appid, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainSetFibreChannelAppid) { + int ret; + ret = conn->driver->domainSetFibreChannelAppid(domain, appid, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 3a5fa7cb09..912b421632 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -900,6 +900,7 @@ LIBVIRT_7.7.0 { global: virNWFilterDefineXMLFlags; virNetworkDefineXMLFlags; + virDomainSetFibreChannelAppid; } LIBVIRT_7.3.0; # .... define new API here using predicted next version number .... -- 2.31.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b64a86af63..55a0b79f95 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8539,6 +8539,7 @@ static virHypervisorDriver hypervisor_driver = { .domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0 */ .domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */ + .domainSetFibreChannelAppid = remoteDomainSetFibreChannelAppid, /* 7.7.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index df1b126b0c..0a64504d45 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3854,6 +3854,12 @@ struct remote_domain_start_dirty_rate_calc_args { unsigned int flags; }; +struct remote_domain_set_fibre_channel_appid_args { + remote_nonnull_domain dom; + remote_string appid; + unsigned int flags; +}; + /*----- Protocol. -----*/ @@ -6818,5 +6824,11 @@ enum remote_procedure { * @acl: network:write * @acl: network:save */ - REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432 + REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432, + + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_FIBRE_CHANNEL_APPID = 433 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index dad83361fa..731cad112c 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3206,6 +3206,11 @@ struct remote_domain_start_dirty_rate_calc_args { int seconds; u_int flags; }; +struct remote_domain_set_fibre_channel_appid_args { + remote_nonnull_domain dom; + remote_string appid; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3639,4 +3644,5 @@ enum remote_procedure { REMOTE_PROC_NODE_DEVICE_CREATE = 430, REMOTE_PROC_NWFILTER_DEFINE_XML_FLAGS = 431, REMOTE_PROC_NETWORK_DEFINE_XML_FLAGS = 432, + REMOTE_PROC_DOMAIN_SET_FIBRE_CHANNEL_APPID = 433, }; -- 2.31.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 75 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f1f961c51c..e8c819bfb9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20448,6 +20448,80 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom, } +static void +qemuDomainUpdateFibreChannelAppid(virDomainDef *def, + const char *appid) +{ + if (!def->resource) { + def->resource = g_new0(virDomainResourceDef, 1); + } else { + g_free(def->resource->appid); + } + + def->resource->appid = g_strdup(appid); +} + + +static int +qemuDomainSetFibreChannelAppid(virDomainPtr dom, + const char *appid, + unsigned int flags) +{ + virQEMUDriver *driver = dom->conn->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainObj *vm = NULL; + virDomainDef *def = NULL; + virDomainDef *persistentDef = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (appid && virDomainDefResourceAppidValidate(appid) < 0) + return -1; + + if (!(vm = qemuDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainSetFibreChannelAppidEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto endjob; + + if (def) { + qemuDomainObjPrivate *priv = vm->privateData; + + if (virCgroupSetFCAppid(priv->cgroup, appid) < 0) + goto endjob; + + qemuDomainUpdateFibreChannelAppid(def, appid); + + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) + goto endjob; + } + + if (persistentDef) { + qemuDomainUpdateFibreChannelAppid(persistentDef, appid); + + if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) + goto endjob; + } + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -20691,6 +20765,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainAuthorizedSSHKeysSet = qemuDomainAuthorizedSSHKeysSet, /* 6.10.0 */ .domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */ .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ + .domainSetFibreChannelAppid = qemuDomainSetFibreChannelAppid, /* 7.7.0 */ }; -- 2.31.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/manpages/virsh.rst | 14 +++++++++ tools/virsh-domain.c | 65 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 2204bed3bb..5875b523a4 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4515,6 +4515,20 @@ Output the IP address and port number for the VNC display. If the information is not available the processes will provide an exit code of 1. +setappid +-------- + +**Syntax:** + +:: + + setappid domain appid + +Set the Fibre Channel appid string for domain that is used to create VMID to +tag the traffic. Accepts only printable characters and maximal length is 128 +characters. To remove the appid from VM don't pass any appid. + + DEVICE COMMANDS =============== diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e5bd1fdd75..07d9c7f954 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -14070,6 +14070,65 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) } +/* + * "setappid" command + */ +static const vshCmdInfo info_setappid[] = { + {.name = "help", + .data = N_("Set domain Fibre Channel appid") + }, + {.name = "desc", + .data = N_("Set the Fibre Channel appid string for domain that is " + "used to create VMID to tag the traffic. Accepts only " + "printable characters and maximal length is 128 characters. " + "To remove the appid from VM don't pass any appid.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_setappid[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = "appid", + .type = VSH_OT_STRING, + .help = N_("User provided appid string"), + }, + {.name = NULL} +}; + +static bool +cmdSetAppid(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + const char *appid = NULL; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "appid", &appid) < 0) + return false; + + if (virDomainSetFibreChannelAppid(dom, (char *)appid, flags) < 0) + return false; + + return true; +} + + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -14715,5 +14774,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_domdirtyrate_calc, .flags = 0 }, + {.name = "setappid", + .handler = cmdSetAppid, + .opts = opts_setappid, + .info = info_setappid, + .flags = 0 + }, {.name = NULL} }; -- 2.31.1
participants (1)
-
Pavel Hrdina