Re: [libvirt] [ v3 1/4] Schema: Introduce XML schema for network-backed loader and nvram elements.

That's a long $SUBJ.... On 05/21/2018 07:10 AM, Prerna Saxena wrote: ...and these are long lines too. I'm still not convinced you have the "optimal" split of patches. Eventually though this patch should be split up (more on that in patch 2 review). In the long run, I think you should alter <loader> first, then alter <nvram> - and it needs to be done in a slower, more progressive manner. The schema would be included into a patch when the <source> parse/format is done in domain_conf and tests would be added in order to validate (and as has been pointed out to me recently, we should use the genericxml2xmltest for at least the parse/format tests). After reviewing the patches, I'm still a bit "confused" with respect to how one can uniquely "name" or find the <loader> or <nvram> file when using the network <source> protocol. Eventually "something" has to be opened for the loader (e.g. CODE file) and nvram (e.g. VARS file). The current syntax and the changed "file" syntax would allow whatever name is chosen, but since the network syntax may not (e.g. the case of iSCSI) provide a name, but only a /dev/sgN device. So how is the file found? Isn't that something QEMU will need? Consider these two <loader> examples: tests/qemuxml2argvdata/bios-nvram-secure.xml: <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.secboot.fd</loader> tests/qemuxml2argvdata/bios-nvram.xml: <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> or these two <nvram> examples: tests/qemuxml2argvdata/q35-acpi-uefi.xml: <nvram>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram> tests/qemuxml2xmloutdata/bios-nvram.xml: <nvram>/usr/share/OVMF/OVMF_VARS.fd</nvram> With a network path such as your example: + <nvram> + <source protocol='iscsi' name='iqn.1992-01.com.example/0'> + <host name='example.org' port='6000'/> + </source> + </nvram> How do you distinguish? Does that mean the someone has to "know" what's on each exposed LUN? You don't need "backing"... Either the old format is found or some sort of <source...> format is found. If <source> is found, then you know that you have either a "file" or "protocol" attribute and can use that to key or set the StorageSourcePtr->type to FILE or NETWORK prior to calling the storage source parse function. NB: Each of the <group> and </group> entries are indented by 1 space too many. This entire hunk is duplicated for loader and nvram, so make it a <define name="sourceLoaderNvram"> (or something like that) and it can then be used to replace the <ref name="absFilePath"/> with <ref name="sourceLoaderNvram"/> John ...