[PATCH 0/7] Move video default logic to individual drivers
The logic setting a device default should be in the post parse function of individual driver code, not in `src/conf/domain_conf.c`. Rafael Fonseca (7): bhyve: move video default logic to driver libxl: move video default logic to driver vz: openvz: move video default logic to driver vmx: vmware: move video default logic to driver test: move video default logic to driver vbox: move video default logic to driver conf: domain_conf: remove virDomainVideoDefaultType src/bhyve/bhyve_domain.c | 5 ++++ src/conf/domain_conf.c | 52 +++------------------------------ src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/libxl/libxl_domain.c | 60 +++++++++++++++++++++++--------------- src/openvz/openvz_conf.c | 8 +++++ src/test/test_driver.c | 23 +++++++++++++++ src/vbox/vbox_common.c | 16 ++++++++++ src/vmware/vmware_driver.c | 4 +++ src/vmx/vmx.c | 4 +++ src/vz/vz_driver.c | 8 +++++ 11 files changed, 108 insertions(+), 74 deletions(-) -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/bhyve/bhyve_domain.c | 5 +++++ src/conf/domain_conf.c | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index a2a0619846..40ee461b19 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -161,6 +161,11 @@ bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video.type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + dev->data.video.type = VIR_DOMAIN_VIDEO_TYPE_GOP; + } + return 0; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a88a5a744e..957989e848 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15738,7 +15738,6 @@ virDomainVideoDefaultType(const virDomainDef *def) else return VIR_DOMAIN_VIDEO_TYPE_PARALLELS; case VIR_DOMAIN_VIRT_BHYVE: - return VIR_DOMAIN_VIDEO_TYPE_GOP; case VIR_DOMAIN_VIRT_QEMU: case VIR_DOMAIN_VIRT_KQEMU: case VIR_DOMAIN_VIRT_KVM: -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 2 +- src/libxl/libxl_domain.c | 60 ++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 957989e848..04636e8694 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15716,7 +15716,6 @@ virDomainVideoDefaultType(const virDomainDef *def) { switch ((virDomainVirtType)def->virtType) { case VIR_DOMAIN_VIRT_TEST: - case VIR_DOMAIN_VIRT_XEN: if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || def->os.type == VIR_DOMAIN_OSTYPE_LINUX) return VIR_DOMAIN_VIDEO_TYPE_XEN; @@ -15737,6 +15736,7 @@ virDomainVideoDefaultType(const virDomainDef *def) return VIR_DOMAIN_VIDEO_TYPE_VGA; else return VIR_DOMAIN_VIDEO_TYPE_PARALLELS; + case VIR_DOMAIN_VIRT_XEN: case VIR_DOMAIN_VIRT_BHYVE: case VIR_DOMAIN_VIRT_QEMU: case VIR_DOMAIN_VIRT_KQEMU: diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index e3da9f777d..cc53a765e1 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -315,31 +315,43 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; } - if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - int dm_type = libxlDomainGetEmulatorType(def); - - switch (dev->data.video->type) { - case VIR_DOMAIN_VIDEO_TYPE_VGA: - case VIR_DOMAIN_VIDEO_TYPE_XEN: - if (dev->data.video->vram == 0) { - if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) - dev->data.video->vram = 16 * 1024; - else - dev->data.video->vram = 8 * 1024; - } - break; - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - if (dev->data.video->vram == 0) { - if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) - dev->data.video->vram = 8 * 1024; - else - dev->data.video->vram = 4 * 1024; + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { + if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_LINUX) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_XEN; + else if (ARCH_IS_PPC64(def->os.arch)) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + else + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; + } + + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + int dm_type = libxlDomainGetEmulatorType(def); + + switch (dev->data.video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + if (dev->data.video->vram == 0) { + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) + dev->data.video->vram = 16 * 1024; + else + dev->data.video->vram = 8 * 1024; + } + break; + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + if (dev->data.video->vram == 0) { + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) + dev->data.video->vram = 8 * 1024; + else + dev->data.video->vram = 4 * 1024; + } + break; + case VIR_DOMAIN_VIDEO_TYPE_QXL: + if (dev->data.video->vram == 0) + dev->data.video->vram = 128 * 1024; + break; } - break; - case VIR_DOMAIN_VIDEO_TYPE_QXL: - if (dev->data.video->vram == 0) - dev->data.video->vram = 128 * 1024; - break; } } -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 4 ---- src/openvz/openvz_conf.c | 8 ++++++++ src/vz/vz_driver.c | 8 ++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04636e8694..e6a3500b7a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15732,10 +15732,6 @@ virDomainVideoDefaultType(const virDomainDef *def) case VIR_DOMAIN_VIRT_VZ: case VIR_DOMAIN_VIRT_PARALLELS: - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) - return VIR_DOMAIN_VIDEO_TYPE_VGA; - else - return VIR_DOMAIN_VIDEO_TYPE_PARALLELS; case VIR_DOMAIN_VIRT_XEN: case VIR_DOMAIN_VIRT_BHYVE: case VIR_DOMAIN_VIRT_QEMU: diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 1d60afae93..78547b8b28 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -1117,6 +1117,14 @@ openvzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return -1; } + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + else + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_PARALLELS; + } + return 0; } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 6605247dd9..d882b91def 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -281,6 +281,14 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, def->os.type == VIR_DOMAIN_OSTYPE_HVM) dev->data.net->model = VIR_DOMAIN_NET_MODEL_E1000; + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + else + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_PARALLELS; + } + return 0; } -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 2 -- src/vmware/vmware_driver.c | 4 ++++ src/vmx/vmx.c | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e6a3500b7a..53bc791e10 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15728,8 +15728,6 @@ virDomainVideoDefaultType(const virDomainDef *def) return VIR_DOMAIN_VIDEO_TYPE_VBOX; case VIR_DOMAIN_VIRT_VMWARE: - return VIR_DOMAIN_VIDEO_TYPE_VMVGA; - case VIR_DOMAIN_VIRT_VZ: case VIR_DOMAIN_VIRT_PARALLELS: case VIR_DOMAIN_VIRT_XEN: diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 32c81b13a0..d5dd6e4f5e 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -137,6 +137,10 @@ vmwareDomainDeviceDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED, void *parseOpaque G_GNUC_UNUSED) { + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VMVGA; + return 0; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 6c6ef7acf3..b1fd1181eb 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -548,6 +548,10 @@ virVMXDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, void *opaque G_GNUC_UNUSED, void *parseOpaque G_GNUC_UNUSED) { + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VMVGA; + return 0; } -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 10 +--------- src/test/test_driver.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 53bc791e10..53fd13e80f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15715,18 +15715,10 @@ int virDomainVideoDefaultType(const virDomainDef *def) { switch ((virDomainVirtType)def->virtType) { - case VIR_DOMAIN_VIRT_TEST: - if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || - def->os.type == VIR_DOMAIN_OSTYPE_LINUX) - return VIR_DOMAIN_VIDEO_TYPE_XEN; - else if (ARCH_IS_PPC64(def->os.arch)) - return VIR_DOMAIN_VIDEO_TYPE_VGA; - else - return VIR_DOMAIN_VIDEO_TYPE_CIRRUS; - case VIR_DOMAIN_VIRT_VBOX: return VIR_DOMAIN_VIDEO_TYPE_VBOX; + case VIR_DOMAIN_VIRT_TEST: case VIR_DOMAIN_VIRT_VMWARE: case VIR_DOMAIN_VIRT_VZ: case VIR_DOMAIN_VIRT_PARALLELS: diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 73fe1ad6ce..7759847c2d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -407,6 +407,28 @@ testDomainObjPrivateAlloc(void *opaque) } +static int +testDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, + const virDomainDef *def G_GNUC_UNUSED, + unsigned int parseFlags G_GNUC_UNUSED, + void *opaque G_GNUC_UNUSED, + void *parseOpaque G_GNUC_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_LINUX) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_XEN; + else if (ARCH_IS_PPC64(def->os.arch)) + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + else + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; + } + + return 0; +} + + static void testDomainObjPrivateFree(void *data) { @@ -431,6 +453,7 @@ testDriverNew(void) VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, + .devicesPostParseCallback = testDomainDevicesDefPostParse, .defArch = VIR_ARCH_I686, }; virDomainXMLPrivateDataCallbacks privatecb = { -- 2.25.1
The logic setting a device default should be in the post parse function of individual driver code. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 2 -- src/vbox/vbox_common.c | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 53fd13e80f..665bb10b27 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15716,8 +15716,6 @@ virDomainVideoDefaultType(const virDomainDef *def) { switch ((virDomainVirtType)def->virtType) { case VIR_DOMAIN_VIRT_VBOX: - return VIR_DOMAIN_VIDEO_TYPE_VBOX; - case VIR_DOMAIN_VIRT_TEST: case VIR_DOMAIN_VIRT_VMWARE: case VIR_DOMAIN_VIRT_VZ: diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 618663952a..e98ae04ec0 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -62,9 +62,25 @@ static virMutex vbox_driver_lock = VIR_MUTEX_INITIALIZER; static vboxDriverPtr vbox_driver; static vboxDriverPtr vboxDriverObjNew(void); +static int +vboxDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, + const virDomainDef *def G_GNUC_UNUSED, + unsigned int parseFlags G_GNUC_UNUSED, + void *opaque G_GNUC_UNUSED, + void *parseOpaque G_GNUC_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VBOX; + } + + return 0; +} + static virDomainDefParserConfig vboxDomainDefParserConfig = { .macPrefix = { 0x08, 0x00, 0x27 }, .features = VIR_DOMAIN_DEF_FEATURE_NAME_SLASH, + .devicesPostParseCallback = vboxDomainDevicesDefPostParse, }; static virCapsPtr -- 2.25.1
The logic has been moved to the individual drivers. Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> --- src/conf/domain_conf.c | 35 ++++------------------------------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 665bb10b27..27bc5a797b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15711,32 +15711,6 @@ virDomainVideoDefaultRAM(const virDomainDef *def, } -int -virDomainVideoDefaultType(const virDomainDef *def) -{ - switch ((virDomainVirtType)def->virtType) { - case VIR_DOMAIN_VIRT_VBOX: - case VIR_DOMAIN_VIRT_TEST: - case VIR_DOMAIN_VIRT_VMWARE: - case VIR_DOMAIN_VIRT_VZ: - case VIR_DOMAIN_VIRT_PARALLELS: - case VIR_DOMAIN_VIRT_XEN: - case VIR_DOMAIN_VIRT_BHYVE: - case VIR_DOMAIN_VIRT_QEMU: - case VIR_DOMAIN_VIRT_KQEMU: - case VIR_DOMAIN_VIRT_KVM: - case VIR_DOMAIN_VIRT_LXC: - case VIR_DOMAIN_VIRT_UML: - case VIR_DOMAIN_VIRT_OPENVZ: - case VIR_DOMAIN_VIRT_HYPERV: - case VIR_DOMAIN_VIRT_PHYP: - case VIR_DOMAIN_VIRT_NONE: - case VIR_DOMAIN_VIRT_LAST: - default: - return VIR_DOMAIN_VIDEO_TYPE_DEFAULT; - } -} - static virDomainVideoAccelDefPtr virDomainVideoAccelDefParseXML(xmlNodePtr node) { @@ -15854,7 +15828,6 @@ static virDomainVideoDefPtr virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr node, xmlXPathContextPtr ctxt, - const virDomainDef *dom, unsigned int flags) { virDomainVideoDefPtr def; @@ -15925,7 +15898,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } } else { - def->type = virDomainVideoDefaultType(dom); + def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT; } if (driver_name) { @@ -16871,7 +16844,7 @@ virDomainDeviceDefParse(const char *xmlStr, break; case VIR_DOMAIN_DEVICE_VIDEO: if (!(dev->data.video = virDomainVideoDefParseXML(xmlopt, node, - ctxt, def, flags))) + ctxt, flags))) return NULL; break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -21633,7 +21606,7 @@ virDomainDefParseXML(xmlDocPtr xml, ssize_t insertAt = -1; if (!(video = virDomainVideoDefParseXML(xmlopt, nodes[i], - ctxt, def, flags))) + ctxt, flags))) goto error; if (video->primary) { @@ -24314,7 +24287,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt) if (!(video = virDomainVideoDefNew(xmlopt))) goto cleanup; - video->type = virDomainVideoDefaultType(def); + video->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT; if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0) goto cleanup; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b7c31eb62f..33875d942f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3364,7 +3364,6 @@ int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs); int virDomainFSIndexByName(virDomainDefPtr def, const char *name); virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i); -int virDomainVideoDefaultType(const virDomainDef *def); unsigned int virDomainVideoDefaultRAM(const virDomainDef *def, const virDomainVideoType type); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c9f0da2bf9..3f032c7963 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -633,7 +633,6 @@ virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; virDomainUSBDeviceDefForeach; virDomainVideoDefaultRAM; -virDomainVideoDefaultType; virDomainVideoDefClear; virDomainVideoDefFree; virDomainVideoDefNew; -- 2.25.1
On 24. 3. 2020 17:14, Rafael Fonseca wrote:
The logic setting a device default should be in the post parse function of individual driver code, not in `src/conf/domain_conf.c`.
Rafael Fonseca (7): bhyve: move video default logic to driver libxl: move video default logic to driver vz: openvz: move video default logic to driver vmx: vmware: move video default logic to driver test: move video default logic to driver vbox: move video default logic to driver conf: domain_conf: remove virDomainVideoDefaultType
src/bhyve/bhyve_domain.c | 5 ++++ src/conf/domain_conf.c | 52 +++------------------------------ src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/libxl/libxl_domain.c | 60 +++++++++++++++++++++++--------------- src/openvz/openvz_conf.c | 8 +++++ src/test/test_driver.c | 23 +++++++++++++++ src/vbox/vbox_common.c | 16 ++++++++++ src/vmware/vmware_driver.c | 4 +++ src/vmx/vmx.c | 4 +++ src/vz/vz_driver.c | 8 +++++ 11 files changed, 108 insertions(+), 74 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Nice cleanup. Michal
participants (2)
-
Michal Prívozník -
Rafael Fonseca