[PATCH] Introduce network-backed NVRAM

Rohit Kumar rohit.kumar3 at nutanix.com
Mon Mar 28 18:36:12 UTC 2022


Thanks a lot for taking the time to review it !

On 28/03/22 6:35 pm, Peter Krempa wrote:
> On Sun, Mar 20, 2022 at 21:28:13 -0700, Rohit Kumar wrote:
>> Libvirt domain XML allows only local filepaths to specify a NVRAM
>> element. Since VMs can move across different hosts, it should be
>> possibe to allocate NVRAM disks on network storage for uninturrupted
>> access.
>>
>> This Patch extends the NVRAM disk elements to be described as
>> virStorageSource* elements.
>>
>> Sample XML with new annotation:
>>
>> <nvram>
> This should have a 'type' attribute rather than blindly checking the
> fields.
Sure, I will add this attribute for network/file type in next patch.
>>    <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'>
>>      <host name='example.com' port='6000'/>
>>    </source>
>> </nvram>
>>
>> or
>>
>> <nvram>
>>    <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/>
>> </nvram>
>>
>> Signed-off-by: Prerna Saxena <prerna.saxena at nutanix.com>
>> Signed-off-by: Florian Schmidt <flosch at nutanix.com>
>> Signed-off-by: Rohit Kumar <rohit.kumar3 at nutanix.com>
>> ---
>>   docs/schemas/domaincommon.rng                 | 72 +++++++++------
>>   src/conf/domain_conf.c                        | 90 +++++++++++++++----
>>   src/conf/domain_conf.h                        |  2 +-
>>   src/qemu/qemu_cgroup.c                        |  3 +-
>>   src/qemu/qemu_command.c                       |  5 +-
>>   src/qemu/qemu_domain.c                        | 23 +++--
>>   src/qemu/qemu_driver.c                        |  5 +-
>>   src/qemu/qemu_firmware.c                      | 17 ++--
>>   src/qemu/qemu_namespace.c                     |  5 +-
>>   src/qemu/qemu_process.c                       |  5 +-
>>   src/security/security_dac.c                   |  6 +-
>>   src/security/security_selinux.c               |  6 +-
>>   src/security/virt-aa-helper.c                 |  5 +-
>>   src/vbox/vbox_common.c                        |  8 +-
>>   .../qemuxml2argvdata/bios-nvram-network.args  | 35 ++++++++
>>   tests/qemuxml2argvdata/bios-nvram-network.xml | 40 +++++++++
>>   tests/qemuxml2argvtest.c                      |  1 +
> So this patch is doing too much. You'll have to split it. I'll note how
> below.
Thanks! Most of the changes are interdependent, so I found a bit hard to 
saperate it.
> You are completely missing documentation for the new syntax in
> docs/formatdomain.rst.
Sure, I will update it. Thanks for pointing this out.
>
>>   17 files changed, 256 insertions(+), 72 deletions(-)
>>   create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args
>>   create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 9c1b64a644..a25c84a0b7 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -330,7 +330,17 @@
>>                 </attribute>
>>               </optional>
>>               <optional>
>> -              <ref name="absFilePath"/>
>> +              <choice>
>> +                <group>
>> +                  <ref name="absFilePath"/>
>> +                </group>
>> +                <group>
>> +                  <ref name="diskSourceFileElement"/>
>> +                </group>
>> +                <group>
>> +                  <ref name="diskSourceNetworkElement"/>
>> +                </group>
>> +              </choice>
>>               </optional>
>>             </element>
>>           </optional>
>> @@ -1714,6 +1724,31 @@
>>       </choice>
>>     </define>
>>   
>> +  <define name="diskSourceFileElement">
>> +    <element name="source">
>> +      <interleave>
>> +        <optional>
>> +          <attribute name="file">
>> +            <choice>
>> +                <ref name="absFilePath"/>
>> +                <ref name="vmwarePath"/>
>> +            </choice>
>> +          </attribute>
>> +        </optional>
>> +        <ref name="diskSourceCommon"/>
>> +        <optional>
>> +          <ref name="storageStartupPolicy"/>
>> +        </optional>
>> +        <optional>
>> +          <ref name="encryption"/>
>> +        </optional>
>> +        <zeroOrMore>
>> +          <ref name="devSeclabel"/>
>> +        </zeroOrMore>
>> +      </interleave>
>> +    </element>
>> +  </define>
>> +
>>     <define name="diskSourceFile">
>>       <optional>
>>         <attribute name="type">
>> @@ -1721,28 +1756,7 @@
>>         </attribute>
>>       </optional>
>>       <optional>
>> -      <element name="source">
>> -        <interleave>
>> -          <optional>
>> -            <attribute name="file">
>> -              <choice>
>> -                <ref name="absFilePath"/>
>> -                <ref name="vmwarePath"/>
>> -              </choice>
>> -            </attribute>
>> -          </optional>
>> -          <ref name="diskSourceCommon"/>
>> -          <optional>
>> -            <ref name="storageStartupPolicy"/>
>> -          </optional>
>> -          <optional>
>> -            <ref name="encryption"/>
>> -          </optional>
>> -          <zeroOrMore>
>> -            <ref name="devSeclabel"/>
>> -          </zeroOrMore>
>> -        </interleave>
>> -      </element>
>> +      <ref name="diskSourceFileElement"/>
>>       </optional>
>>     </define>
>>   
>> @@ -2137,10 +2151,7 @@
>>       </element>
>>     </define>
>>   
>> -  <define name="diskSourceNetwork">
>> -    <attribute name="type">
>> -      <value>network</value>
>> -    </attribute>
>> +  <define name="diskSourceNetworkElement">
>>       <choice>
>>         <ref name="diskSourceNetworkProtocolNBD"/>
>>         <ref name="diskSourceNetworkProtocolGluster"/>
>> @@ -2155,6 +2166,13 @@
>>       </choice>
>>     </define>
>>   
>> +  <define name="diskSourceNetwork">
>> +    <attribute name="type">
>> +      <value>network</value>
>> +    </attribute>
>> +    <ref name="diskSourceNetworkElement"/>
>> +  </define>
>> +
>>     <define name="diskSourceVolume">
>>       <attribute name="type">
>>         <value>volume</value>
>
> Please separate the cleanups (moving of definitions under the 'define'
> element into a separate patch.
Ack., thanks !
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index e0dfc9e45f..5fcc09db05 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -3535,7 +3535,7 @@ virDomainLoaderDefFree(virDomainLoaderDef *loader)
>>           return;
>>   
>>       g_free(loader->path);
>> -    g_free(loader->nvram);
>> +    virObjectUnref(loader->nvram);
>>       g_free(loader->nvramTemplate);
>>       g_free(loader);
>>   }
>> @@ -17834,6 +17834,37 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
>>   }
>>   
>>   
>> +static int
>> +virDomainNvramDefParseXML(virStorageSource *nvram,
>> +                          xmlXPathContextPtr ctxt,
>> +                          virDomainXMLOption *opt,
>> +                          unsigned int flags)
>> +{
>> +    g_autofree char *srcTypeFile = NULL;
>> +    g_autofree char *srcTypeNetwork = NULL;
>> +    xmlNodePtr source;
>> +
>> +    srcTypeFile =  virXPathString("string(./os/nvram/source/@file)", ctxt);
>> +    srcTypeNetwork =  virXPathString("string(./os/nvram/source/@protocol)", ctxt);
>> +
>> +    if (!srcTypeFile && !srcTypeNetwork) {
>> +        nvram->type = VIR_STORAGE_TYPE_FILE;
>> +        nvram->path = virXPathString("string(./os/nvram[1])", ctxt);
>> +        return 0;
>> +    } else {
>> +        if (srcTypeFile) {
>> +            nvram->type = VIR_STORAGE_TYPE_FILE;
>> +        } else {
>> +            nvram->type = VIR_STORAGE_TYPE_NETWORK;
>> +        }
>> +        source = virXPathNode("./os/nvram/source[1]", ctxt);
>> +        if (!source)
>> +            return -1;
> virXPathNode does not report an error but you return it, thus the code
> would not report anything.
Right, If source tag is not present, we should report an error before 
returning. Will Update this as well.
>
>> +        return virDomainStorageSourceParse(source, ctxt, nvram, flags, opt);
>> +    }
>> +
>> +}
>> +
>>   static int
>>   virDomainSchedulerParseCommonAttrs(xmlNodePtr node,
>>                                      virProcessSchedPolicy *policy,
>> @@ -18219,7 +18250,9 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
>>   
>>   static int
>>   virDomainDefParseBootLoaderOptions(virDomainDef *def,
>> -                                   xmlXPathContextPtr ctxt)
>> +                                   xmlXPathContextPtr ctxt,
>> +                                   virDomainXMLOption *xmlopt,
>> +                                   unsigned int flags)
>>   {
>>       xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt);
>>       const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
>> @@ -18234,7 +18267,14 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
>>                                      fwAutoSelect) < 0)
>>           return -1;
>>   
>> -    def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt);
>> +    if (virXPathNode("./os/nvram[1]", ctxt)) {
>> +        def->os.loader->nvram = g_new0(virStorageSource, 1);
>> +
>> +        if (virDomainNvramDefParseXML(def->os.loader->nvram,
>> +                                      ctxt, xmlopt, flags) < 0)
>> +            return -1;
>> +    }
>> +
>>       if (!fwAutoSelect)
>>           def->os.loader->nvramTemplate = virXPathString("string(./os/nvram[1]/@template)", ctxt);
>>   
>> @@ -18288,7 +18328,9 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def,
>>   
>>   static int
>>   virDomainDefParseBootOptions(virDomainDef *def,
>> -                             xmlXPathContextPtr ctxt)
>> +                             xmlXPathContextPtr ctxt,
>> +                             virDomainXMLOption *xmlopt,
>> +                             unsigned int flags)
>>   {
>>       /*
>>        * Booting options for different OS types....
>> @@ -18306,7 +18348,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
>>           if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0)
>>               return -1;
>>   
>> -        if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0)
>> +        if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)
>>               return -1;
>>   
>>           if (virDomainDefParseBootAcpiOptions(def, ctxt) < 0)
>> @@ -18322,7 +18364,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
>>       case VIR_DOMAIN_OSTYPE_UML:
>>           virDomainDefParseBootKernelOptions(def, ctxt);
>>   
>> -        if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0)
>> +        if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)
>>               return -1;
>>   
>>           break;
>> @@ -19606,7 +19648,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
>>       if (virDomainDefClockParse(def, ctxt) < 0)
>>           return NULL;
>>   
>> -    if (virDomainDefParseBootOptions(def, ctxt) < 0)
>> +    if (virDomainDefParseBootOptions(def, ctxt, xmlopt, flags) < 0)
>>           return NULL;
>>   
>>       /* analysis of the disk devices */
>> @@ -26899,7 +26941,9 @@ virDomainHugepagesFormat(virBuffer *buf,
>>   
>>   static void
>>   virDomainLoaderDefFormat(virBuffer *buf,
>> -                         virDomainLoaderDef *loader)
>> +                         virDomainLoaderDef *loader,
>> +                         virDomainXMLOption *xmlopt,
>> +                         unsigned int flags)
>>   {
>>       const char *readonly = virTristateBoolTypeToString(loader->readonly);
>>       const char *secure = virTristateBoolTypeToString(loader->secure);
>> @@ -26921,13 +26965,27 @@ virDomainLoaderDefFormat(virBuffer *buf,
>>       else
>>           virBufferAddLit(buf, "/>\n");
>>   
>> -    if (loader->nvram || loader->nvramTemplate) {
>> -        virBufferAddLit(buf, "<nvram");
>> -        virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate);
>> -        if (loader->nvram)
>> -            virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram);
>> -        else
>> -            virBufferAddLit(buf, "/>\n");
>> +    if (loader->nvram) {
>> +        if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) {
>> +            virBufferAddLit(buf, "<nvram");
>> +            virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate);
>> +            if (loader->nvram->path)
>> +                virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram->path);
>> +            else
>> +                virBufferAddLit(buf, "/>\n");
>> +        } else {
>> +            virBufferAddLit(buf, "<nvram");
>> +            virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate);
>> +            virBufferAdjustIndent(buf, 2);
>> +            if (virDomainDiskSourceFormat(buf, loader->nvram, "source", 0,
>> +                                          0, false, flags, true, xmlopt) < 0) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("Cannot format NVRAM source"));
> 'virDomainDiskSourceFormat' is already reporting an error so you must
> not overwrite it ...
Yes, I will remove it.
>> +                return;
> ... also you must propagate it appropriately.
Ack.
>
>> +            }
>> +            virBufferAdjustIndent(buf, -2);
>> +            virBufferAddLit(buf, "</nvram>\n");
> I'll post a patch refactoring this function to use virXMLFormatElement
> which will greatly clean up your addition. Make sure to rebase it on top
> of it.
Sure, thanks !
>
>> +        }
>>       }
>>   }
>>   
> [...]
>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index c836799888..649a6f5b55 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9583,6 +9583,7 @@ qemuBuildDomainLoaderPflashCommandLine(virCommand *cmd,
>>                                         virQEMUCaps *qemuCaps)
>>   {
>>       g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>> +    g_autofree char *nvramPath = NULL;
>>       int unit = 0;
>>   
>>       if (loader->secure == VIR_TRISTATE_BOOL_YES) {
>> @@ -9610,8 +9611,10 @@ qemuBuildDomainLoaderPflashCommandLine(virCommand *cmd,
>>       virCommandAddArgBuffer(cmd, &buf);
>>   
>>       if (loader->nvram) {
>> +        if (qemuGetDriveSourceString(loader->nvram, NULL, &nvramPath) < 0)
>> +            return;
> NACK to this bit. We should only allow the new feature when qemu
> supports VIR_QEMU_CAPS_BLOCKDEV, thus no change to this function should
> be needed.
>
> Add the check into qemu_validate.c into the appropriate place.
Ack. Thanks!
>
> Nobody is reallistically going to keep maintaining support for old qemu.
yes, true !
>
>>           virBufferAddLit(&buf, "file=");
>> -        virQEMUBuildBufferEscapeComma(&buf, loader->nvram);
>> +        virQEMUBuildBufferEscapeComma(&buf, nvramPath);
>>           virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit);
>>   
>>           virCommandAddArg(cmd, "-drive");
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 7180ae616b..303d9661a4 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -4452,8 +4452,13 @@ qemuDomainDefPostParse(virDomainDef *def,
>>       }
>>   
>>       if (virDomainDefHasOldStyleROUEFI(def) &&
>> -        !def->os.loader->nvram)
>> -        qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram);
>> +        (!def->os.loader->nvram || !def->os.loader->nvram->path)) {
>> +
>> +        if (!def->os.loader->nvram)
>> +            def->os.loader->nvram = g_new0(virStorageSource, 1);
>> +        def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
>> +        qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram->path);
>> +    }
>>   
>>       if (qemuDomainDefAddDefaultDevices(driver, def, qemuCaps) < 0)
>>           return -1;
>> @@ -11133,15 +11138,23 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm)
>>       pflash0->nodeformat = g_strdup("libvirt-pflash0-format");
>>       pflash0->nodestorage = g_strdup("libvirt-pflash0-storage");
>>   
>> -
>>       if (def->os.loader->nvram) {
>>           pflash1 = virStorageSourceNew();
>> -        pflash1->type = VIR_STORAGE_TYPE_FILE;
>>           pflash1->format = VIR_STORAGE_FILE_RAW;
>> -        pflash1->path = g_strdup(def->os.loader->nvram);
>> +        pflash1->path = g_strdup(def->os.loader->nvram->path);
>>           pflash1->readonly = false;
>>           pflash1->nodeformat = g_strdup("libvirt-pflash1-format");
>>           pflash1->nodestorage = g_strdup("libvirt-pflash1-storage");
>> +
>> +        if (def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) {
>> +             pflash1->type = VIR_STORAGE_TYPE_FILE;
>> +        } else if (def->os.loader->nvram->type == VIR_STORAGE_TYPE_NETWORK) {
>> +            pflash1->protocol = def->os.loader->nvram->protocol;
>> +            pflash1->hosts =  g_new0(virStorageNetHostDef, 1);
>> +            pflash1->nhosts = 1;
>> +            pflash1->hosts[0].name = def->os.loader->nvram->hosts[0].name;
>> +            pflash1->hosts[0].port = def->os.loader->nvram->hosts[0].port;
> since both 'def->os.loader->nvram' and pflash1 are 'virStorageSource'
> you should use virStorageSourceCopy instead of this.
Ack.
>
>
>> +        }
>>       }
>>   
>>       priv->pflash0 = g_steal_pointer(&pflash0);
>
> [...]
>
>> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
>> index 51223faadf..fa99c7d217 100644
>> --- a/src/qemu/qemu_firmware.c
>> +++ b/src/qemu/qemu_firmware.c
>> @@ -1192,13 +1192,16 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
>>           VIR_FREE(def->os.loader->nvramTemplate);
>>           def->os.loader->nvramTemplate = g_strdup(flash->nvram_template.filename);
>>   
>> -        if (!def->os.loader->nvram)
>> -            qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram);
>> +        if (!def->os.loader->nvram) {
>> +            def->os.loader->nvram = g_new0(virStorageSource, 1);
>> +            def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
>> +            qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram->path);
>> +        }
>>   
>>           VIR_DEBUG("decided on firmware '%s' template '%s' NVRAM '%s'",
>>                     def->os.loader->path,
>>                     def->os.loader->nvramTemplate,
>> -                  def->os.loader->nvram);
>> +                  def->os.loader->nvram->path);
>>           break;
>>   
>>       case QEMU_FIRMWARE_DEVICE_KERNEL:
>> @@ -1364,8 +1367,12 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
>>            * its path in domain XML) but no template for NVRAM was
>>            * specified and the varstore doesn't exist ... */
>>           if (!virDomainDefHasOldStyleROUEFI(def) ||
>> -            def->os.loader->nvramTemplate ||
>> -            (!reset_nvram && virFileExists(def->os.loader->nvram)))
>> +            def->os.loader->nvramTemplate  ||
>> +            (def->os.loader->nvram &&
>> +             !reset_nvram && ((def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE
>> +             && virFileExists(def->os.loader->nvram->path)) ||
>> +             (def->os.loader->nvram->type == VIR_STORAGE_TYPE_NETWORK &&
>> +             def->os.loader->nvram->path))))
> This condition got a bit too crazy ... please factor out the inner
> clause you've added into a sub-condition.
Yes, sure.
>>               return 0;
>>   
>>           /* ... then we want to consult JSON FW descriptors first,
> [...]
>
>
>> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
>> index acd18494d3..d6a482d28b 100644
>> --- a/src/vbox/vbox_common.c
>> +++ b/src/vbox/vbox_common.c
>> @@ -994,10 +994,10 @@ vboxSetBootDeviceOrder(virDomainDef *def, struct _vboxDriver *data,
>>       VIR_DEBUG("def->os.cmdline          %s", def->os.cmdline);
>>       VIR_DEBUG("def->os.root             %s", def->os.root);
>>       if (def->os.loader) {
>> -        VIR_DEBUG("def->os.loader->path     %s", def->os.loader->path);
>> -        VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly);
>> -        VIR_DEBUG("def->os.loader->type     %d", def->os.loader->type);
>> -        VIR_DEBUG("def->os.loader->nvram    %s", def->os.loader->nvram);
>> +        VIR_DEBUG("def->os.loader->path         %s", def->os.loader->path);
>> +        VIR_DEBUG("def->os.loader->readonly     %d", def->os.loader->readonly);
>> +        VIR_DEBUG("def->os.loader->type         %d", def->os.loader->type);
>> +        VIR_DEBUG("def->os.loader->nvram->path  %s", def->os.loader->nvram->path);
> These seem to be aligning with the block above as well, but IMO we
> should not do any alignment here. I'll post a patch removing the
> whitespace, so please rebase to it.
yes, Thanks.
>
>>       }
>>       VIR_DEBUG("def->os.bootloader       %s", def->os.bootloader);
>>       VIR_DEBUG("def->os.bootloaderArgs   %s", def->os.bootloaderArgs);
>> diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args
>> new file mode 100644
>> index 0000000000..05075b45de
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/bios-nvram-network.args
>> @@ -0,0 +1,35 @@
>> +LC_ALL=C \
>> +PATH=/bin \
>> +HOME=/tmp/lib/domain--1-test-bios \
>> +USER=test \
>> +LOGNAME=test \
>> +XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \
>> +XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \
>> +XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \
>> +QEMU_AUDIO_DRV=none \
>> +/usr/bin/qemu-system-x86_64 \
>> +-name guest=test-bios,debug-threads=on \
>> +-S \
>> +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test-bios/master-key.aes \
>> +-machine pc,usb=off,dump-guest-core=off \
>> +-accel tcg \
>> +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
>> +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example,if=pflash,format=raw,unit=1 \
> As noted below we need at least one test case which uses -blockdev for
> this which should be used by modern qemu.
Ok, I will update the test case for blockdev cap. If we are introducing 
this feature when blockdev is supported, then this test case would not 
be needed anymore.
>
>> +-m 1024 \
>> +-realtime mlock=off \
>> +-smp 1,sockets=1,cores=1,threads=1 \
>> +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \
>> +-display none \
>> +-no-user-config \
>> +-nodefaults \
>> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,server=on,wait=off \
>> +-mon chardev=charmonitor,id=monitor,mode=control \
>> +-rtc base=utc \
>> +-no-shutdown \
>> +-boot menu=on,strict=on \
>> +-usb \
>> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
>> +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
>> +-device usb-tablet,id=input0,bus=usb.0,port=1 \
>> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
>> +-msg timestamp=on
> [...]
>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index e7fecb24d3..7ec45ec74b 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -1189,6 +1189,7 @@ mymain(void)
>>               QEMU_CAPS_DEVICE_ISA_SERIAL);
>>       DO_TEST_NOCAPS("bios-nvram");
>>       DO_TEST_PARSE_ERROR_NOCAPS("bios-nvram-no-path");
>> +    DO_TEST_NOCAPS("bios-nvram-network");
> For new tests, please always use 'DO_TEST_CAPS_LATEST' or
> DO_TEST_CAPS_VER for older versions. New use of DO_TEST_NOCAPS is not
> desirable.
Ack. Thanks !
>
>>       DO_TEST_CAPS_LATEST("bios-nvram-rw");
>>       DO_TEST_CAPS_LATEST("bios-nvram-rw-implicit");
>>       DO_TEST("bios-nvram-secure",
>> -- 
>> 2.25.1
>>



More information about the libvir-list mailing list