[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH virt-manager] details: Add UI for enabling UEFI via 'customize before install'

On 21.09.2014 16:58, Laszlo Ersek wrote:
Hi Cole and Michal,

I'm attaching three patches:

(2) Unfortunately, even libvirtd needs to be modified, in addition.

My patch for (1) *does* pass VIR_DOMAIN_UNDEFINE_NVRAM to libvirtd (I
verified that with gdb), but libvirtd doesn't act upon it correctly.
Namely, in the libvirtd source code, in qemuDomainUndefineFlags(),
commit 273b6581 added:

     if (!virDomainObjIsActive(vm) &&
         vm->def->os.loader && vm->def->os.loader->nvram &&
         virFileExists(vm->def->os.loader->nvram)) {
         if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                            _("cannot delete inactive domain with nvram"));
             goto cleanup;

         if (unlink(vm->def->os.loader->nvram) < 0) {
                                  _("failed to remove nvram: %s"),
             goto cleanup;

Here "vm->def->os.loader->nvram" evaluates to NULL:

   6468        if (!virDomainObjIsActive(vm) &&
   6469            vm->def->os.loader && vm->def->os.loader->nvram &&
   6470            virFileExists(vm->def->os.loader->nvram)) {
   6471            if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
   6472                virReportError(VIR_ERR_OPERATION_INVALID, "%s",

   (gdb) print vm->def->os.loader
   $1 = (virDomainLoaderDefPtr) 0x7ff59c00bf20

   (gdb) print vm->def->os.loader->nvram
   $2 = 0x0

I thought that the auto-generation of the nvram pathname (internal to
libvirtd, ie. not visible in the XML file) would occur on the undefine
path as well. Apparently this is not the case.

Indeed, the only call to qemuPrepareNVRAM() is from qemuProcessStart().

Michal, would it be possible generate the *pathname* of the separate
varstore in qemuDomainUndefineFlags() too?

Please see the second attached patch; it works for me. (This patch is
best looked at with "git diff -b" (or "git show -b").)

The original problem is, that once the path is generated, the config XML under /etc/libvirt/qemu/ is not updated as it need to. Having said that, this patch is then not needed anymore.

(3) I just realized that a domain's name can change during its lifetime.

Well, the only moment that we support that is migration. Otherwise the name can't be changed (it's not only varstore path that's derived from domain's name, consider snapshots, socket names, ...). Having said that, the current code is not safe, because the domain name may contain characters not supported by the FS mounted on /var/lib/libvirt/qemu/nvram. But if that's the case users can just provide the acceptable nvram path. And that's the case in migration too - users can pass not only new domain name but new domain XML too. And the XML can contain changed nvram path to match domain name. Therefore I'm undecided if this patch is needed. I mean, seeing bare UUIDs under /var/.../nvram directory doesn't make the life easier for admins. BTW that's the reason why we still use /etc/libvirt/qemu/$dom_name.xml.

Renaming a domain becomes a problem when the varstore's pathname is
deduced from the domain's name. For this reason, the auto-generation
should use the domain's UUID (which never changes). Michal, what do you
think of the 3rd attached patch?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]