On Fri, Apr 8, 2022 at 11:19 PM Rohit Kumar <rohit.kumar3@nutanix.com> wrote:
Libvirt domain XML currently allows only local filepaths that can be used to specify a NVRAM disk. Since, VMs can migrate across hypervisor hosts, it should be possible to allocate NVRAM disks on network storage for uninterrupted access.
This series extends the NVRAM element to support hosting over network-backed disks, for high availability. It achieves this by embedding virStorageSource pointer for nvram into _virDomainLoaderDef.
It introduces a 'type' attribute for NVRAM element to specify 'file' vs 'network' backed NVRAM.
Changes v1->v2: - Split the patch into smaller patches - Added unit test - Updated the doc - Addressed Peter's comment on v1 (https://listman.redhat.com/archives/libvir-list/2022-March/229684.html)
Ok so without going deeper into the actual change, following are some quick comments based on some of my own experience of introducing new conf options in libvirt: (a) Please update NEWS.rst t to document the new xml attribute support for the next release. This should be the last patch in the series preferrably. (b) Please put patch #2 and #5 together. Also please prefix the first line with "conf:" I think patch #4 should also go together but I will let others comment. (c) It's better that the unit tests (patches #7 and #8) go along with the changes in the same patch. Then when cherry picking the unit tests will be picked along with the change itself. (d) also I have commented separately, your validation patch needs additional unit tests to check the validation actually works.
Rohit Kumar (8): Make NVRAM a virStorageSource type. Add support to parse/format virStorageSource type NVRAM Validate remote store NVRAM Cleanup diskSourceNetwork and diskSourceFile schema Update XML schema to support network backed NVRAM Update NVRAM documentation Add unit test for network backed NVRAM Add unit test to support new 'file' type NVRAM
docs/formatdomain.rst | 43 +++++++-- src/conf/domain_conf.c | 88 ++++++++++++++++--- src/conf/domain_conf.h | 2 +- src/conf/schemas/domaincommon.rng | 80 +++++++++++------ src/qemu/qemu_cgroup.c | 3 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 14 +-- src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_firmware.c | 23 +++-- src/qemu/qemu_namespace.c | 5 +- src/qemu/qemu_process.c | 5 +- src/qemu/qemu_validate.c | 22 +++++ src/security/security_dac.c | 6 +- src/security/security_selinux.c | 6 +- src/security/virt-aa-helper.c | 5 +- src/vbox/vbox_common.c | 2 +- .../bios-nvram-file.x86_64-latest.args | 37 ++++++++ tests/qemuxml2argvdata/bios-nvram-file.xml | 23 +++++ .../bios-nvram-network.x86_64-latest.args | 37 ++++++++ tests/qemuxml2argvdata/bios-nvram-network.xml | 25 ++++++ tests/qemuxml2argvtest.c | 2 + 21 files changed, 360 insertions(+), 75 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml
-- 2.25.1