All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	libvir-list@redhat.com,
	Daniel Henrique Barboza <danielhb@linux.ibm.com>,
	qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com,
	Anthony Perard <anthony.perard@citrix.com>,
	Igor Mammedov <imammedo@redhat.com>,
	dgilbert@redhat.com
Subject: Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target
Date: Tue, 29 May 2018 11:55:45 -0300	[thread overview]
Message-ID: <20180529145545.GB8988@localhost.localdomain> (raw)
In-Reply-To: <87efhwp7jp.fsf@dusky.pond.sub.org>

On Mon, May 28, 2018 at 09:23:54AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
[...]
> > [1] Doing a:
> >   $ git grep 'STR.*machine, "'
> > on libvirt source is enough to find some code demonstrating where
> > query-machines is already lacking today:
[...]
> How can we get from this grep to a list of static or dynamic machine
> type capabilties?

Let's look at the code:


$ git grep -W 'STR.*machine, "'
src/libxl/libxl_capabilities.c=libxlMakeDomainOSCaps(const char *machine,
src/libxl/libxl_capabilities.c-                      virDomainCapsOSPtr os,
src/libxl/libxl_capabilities.c-                      virFirmwarePtr *firmwares,
src/libxl/libxl_capabilities.c-                      size_t nfirmwares)
src/libxl/libxl_capabilities.c-{
src/libxl/libxl_capabilities.c-    virDomainCapsLoaderPtr capsLoader = &os->loader;
src/libxl/libxl_capabilities.c-    size_t i;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c-    os->supported = true;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c:    if (STREQ(machine, "xenpv"))
src/libxl/libxl_capabilities.c-        return 0;

I don't understand why this one is here, but we can find out what
we could add to query-machines to make this unnecessary.


[...]
--
src/libxl/libxl_capabilities.c=libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
src/libxl/libxl_capabilities.c-                            virFirmwarePtr *firmwares,
src/libxl/libxl_capabilities.c-                            size_t nfirmwares)
src/libxl/libxl_capabilities.c-{
src/libxl/libxl_capabilities.c-    virDomainCapsOSPtr os = &domCaps->os;
src/libxl/libxl_capabilities.c-    virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
src/libxl/libxl_capabilities.c-    virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics;
src/libxl/libxl_capabilities.c-    virDomainCapsDeviceVideoPtr video = &domCaps->video;
src/libxl/libxl_capabilities.c-    virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c:    if (STREQ(domCaps->machine, "xenfv"))
src/libxl/libxl_capabilities.c-        domCaps->maxvcpus = HVM_MAX_VCPUS;
src/libxl/libxl_capabilities.c-    else
src/libxl/libxl_capabilities.c-        domCaps->maxvcpus = PV_MAX_VCPUS;

Looks like libvirt isn't using MachineInfo::cpu-max.  libvirt
bug, or workaround for QEMU limitation?


[...]
--
src/libxl/libxl_driver.c=libxlConnectGetDomainCapabilities(virConnectPtr conn,
src/libxl/libxl_driver.c-                                  const char *emulatorbin,
src/libxl/libxl_driver.c-                                  const char *arch_str,
src/libxl/libxl_driver.c-                                  const char *machine,
src/libxl/libxl_driver.c-                                  const char *virttype_str,
src/libxl/libxl_driver.c-                                  unsigned int flags)
src/libxl/libxl_driver.c-{
[...]
src/libxl/libxl_driver.c-    if (machine) {
src/libxl/libxl_driver.c:        if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) {
src/libxl/libxl_driver.c-            virReportError(VIR_ERR_INVALID_ARG, "%s",
src/libxl/libxl_driver.c-                           _("Xen only supports 'xenpv' and 'xenfv' machines"));


Not sure if this should be encoded in QEMU.  accel=xen works with
other PC machines, doesn't it?


[...]
--
src/qemu/qemu_capabilities.c=bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
src/qemu/qemu_capabilities.c-                               const virDomainDef *def)
src/qemu/qemu_capabilities.c-{
src/qemu/qemu_capabilities.c-    /* x86_64 and i686 support PCI-multibus on all machine types
src/qemu/qemu_capabilities.c-     * since forever */
src/qemu/qemu_capabilities.c-    if (ARCH_IS_X86(def->os.arch))
src/qemu/qemu_capabilities.c-        return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-    if (def->os.arch == VIR_ARCH_PPC ||
src/qemu/qemu_capabilities.c-        ARCH_IS_PPC64(def->os.arch)) {
src/qemu/qemu_capabilities.c-        /*
src/qemu/qemu_capabilities.c-         * Usage of pci.0 naming:
src/qemu/qemu_capabilities.c-         *
src/qemu/qemu_capabilities.c-         *    ref405ep: no pci
src/qemu/qemu_capabilities.c-         *       taihu: no pci
src/qemu/qemu_capabilities.c-         *      bamboo: 1.1.0
src/qemu/qemu_capabilities.c-         *       mac99: 2.0.0
src/qemu/qemu_capabilities.c-         *     g3beige: 2.0.0
src/qemu/qemu_capabilities.c-         *        prep: 1.4.0
src/qemu/qemu_capabilities.c-         *     pseries: 2.0.0
src/qemu/qemu_capabilities.c-         *   mpc8544ds: forever
src/qemu/qemu_capabilities.c-         * virtex-m507: no pci
src/qemu/qemu_capabilities.c-         *     ppce500: 1.6.0
src/qemu/qemu_capabilities.c-         */
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        /* We do not store the qemu version in domain status XML.
src/qemu/qemu_capabilities.c-         * Hope the user is using a QEMU new enough to use 'pci.0',
src/qemu/qemu_capabilities.c-         * otherwise the results of this function will be wrong
src/qemu/qemu_capabilities.c-         * for domains already running at the time of daemon
src/qemu/qemu_capabilities.c-         * restart */
src/qemu/qemu_capabilities.c-        if (qemuCaps->version == 0)
src/qemu/qemu_capabilities.c-            return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        if (qemuCaps->version >= 2000000)
src/qemu/qemu_capabilities.c-            return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        if (qemuCaps->version >= 1006000 &&
src/qemu/qemu_capabilities.c:            STREQ(def->os.machine, "ppce500"))
src/qemu/qemu_capabilities.c-            return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        if (qemuCaps->version >= 1004000 &&
src/qemu/qemu_capabilities.c:            STREQ(def->os.machine, "prep"))
src/qemu/qemu_capabilities.c-            return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        if (qemuCaps->version >= 1001000 &&
src/qemu/qemu_capabilities.c:            STREQ(def->os.machine, "bamboo"))
src/qemu/qemu_capabilities.c-            return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c:        if (STREQ(def->os.machine, "mpc8544ds"))
src/qemu/qemu_capabilities.c-            return true;

This is due to the lack of bus information on query-machines.


[...]
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-        return false;
src/qemu/qemu_capabilities.c-    }
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-    /* If 'virt' supports PCI, it supports multibus.
src/qemu/qemu_capabilities.c-     * No extra conditions here for simplicity.
src/qemu/qemu_capabilities.c-     */
src/qemu/qemu_capabilities.c-    if (qemuDomainIsVirt(def))
src/qemu/qemu_capabilities.c-        return true;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-    return false;
src/qemu/qemu_capabilities.c-}
--
src/qemu/qemu_capabilities.c=virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
src/qemu/qemu_capabilities.c-                          const virDomainDef *def)
src/qemu/qemu_capabilities.c-{
src/qemu/qemu_capabilities.c-    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT))
src/qemu/qemu_capabilities.c-        return false;
src/qemu/qemu_capabilities.c-
src/qemu/qemu_capabilities.c-    return qemuDomainIsI440FX(def) ||
src/qemu/qemu_capabilities.c-        qemuDomainIsQ35(def) ||
src/qemu/qemu_capabilities.c:        STREQ(def->os.machine, "isapc");
src/qemu/qemu_capabilities.c-}

Lack of bus information on query-machines?


--
src/qemu/qemu_command.c=qemuBuildMemballoonCommandLine(virCommandPtr cmd,
src/qemu/qemu_command.c-                               const virDomainDef *def,
src/qemu/qemu_command.c-                               virQEMUCapsPtr qemuCaps)
src/qemu/qemu_command.c-{
src/qemu/qemu_command.c-    virBuffer buf = VIR_BUFFER_INITIALIZER;
src/qemu/qemu_command.c-
src/qemu/qemu_command.c:    if (STRPREFIX(def->os.machine, "s390-virtio") &&
src/qemu/qemu_command.c-        virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390) && def->memballoon)
src/qemu/qemu_command.c-        def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE;


Lack of bus information on query-machines?


[...]
--
src/qemu/qemu_domain.c=qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
src/qemu/qemu_domain.c-                               virQEMUCapsPtr qemuCaps)
src/qemu/qemu_domain.c-{
[...]
src/qemu/qemu_domain.c:        if (STREQ(def->os.machine, "isapc")) {
src/qemu/qemu_domain.c-            addDefaultUSB = false;


Lack of bus/defaults information on query-machines?

This whole function is a collection of cases where
bus/device/defaults information is missing on query-machines:

src/qemu/qemu_domain.c-            break;
src/qemu/qemu_domain.c-        }
src/qemu/qemu_domain.c-        if (qemuDomainIsQ35(def)) {
src/qemu/qemu_domain.c-            addPCIeRoot = true;
src/qemu/qemu_domain.c-            addImplicitSATA = true;
[...]
src/qemu/qemu_domain.c-        if (qemuDomainIsI440FX(def))
src/qemu/qemu_domain.c-            addPCIRoot = true;
[...]
src/qemu/qemu_domain.c-        break;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    case VIR_ARCH_ARMV7L:
src/qemu/qemu_domain.c-    case VIR_ARCH_AARCH64:
src/qemu/qemu_domain.c-        addDefaultUSB = false;
src/qemu/qemu_domain.c-        addDefaultMemballoon = false;
src/qemu/qemu_domain.c-        if (qemuDomainIsVirt(def))
src/qemu/qemu_domain.c-            addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX);
src/qemu/qemu_domain.c-        break;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    case VIR_ARCH_PPC64:
src/qemu/qemu_domain.c-    case VIR_ARCH_PPC64LE:
src/qemu/qemu_domain.c-        addPCIRoot = true;
src/qemu/qemu_domain.c-        addDefaultUSBKBD = true;
src/qemu/qemu_domain.c-        addDefaultUSBMouse = true;
src/qemu/qemu_domain.c-        /* For pSeries guests, the firmware provides the same
src/qemu/qemu_domain.c-         * functionality as the pvpanic device, so automatically
src/qemu/qemu_domain.c-         * add the definition if not already present */
src/qemu/qemu_domain.c-        if (qemuDomainIsPSeries(def))
src/qemu/qemu_domain.c-            addPanicDevice = true;
src/qemu/qemu_domain.c-        break;
[...]
--
src/qemu/qemu_domain.c=qemuDomainDefaultNetModel(const virDomainDef *def,
src/qemu/qemu_domain.c-                          virQEMUCapsPtr qemuCaps)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c-    if (ARCH_IS_S390(def->os.arch))
src/qemu/qemu_domain.c-        return "virtio";
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    if (def->os.arch == VIR_ARCH_ARMV7L ||
src/qemu/qemu_domain.c-        def->os.arch == VIR_ARCH_AARCH64) {
src/qemu/qemu_domain.c:        if (STREQ(def->os.machine, "versatilepb"))
src/qemu/qemu_domain.c-            return "smc91c111";
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-        if (qemuDomainIsVirt(def))
src/qemu/qemu_domain.c-            return "virtio";
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-        /* Incomplete. vexpress (and a few others) use this, but not all
src/qemu/qemu_domain.c-         * arm boards */
src/qemu/qemu_domain.c-        return "lan9118";


Lack of bus/defaults information on query-machines?

[...]
--
src/qemu/qemu_domain.c=qemuDomainMachineIsQ35(const char *machine)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c:    return (STRPREFIX(machine, "pc-q35") ||
src/qemu/qemu_domain.c:            STREQ(machine, "q35"));
src/qemu/qemu_domain.c-}

Need to grep for callers of this function.

--
src/qemu/qemu_domain.c=qemuDomainMachineIsI440FX(const char *machine)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c:    return (STREQ(machine, "pc") ||
src/qemu/qemu_domain.c:            STRPREFIX(machine, "pc-0.") ||
src/qemu/qemu_domain.c:            STRPREFIX(machine, "pc-1.") ||
src/qemu/qemu_domain.c:            STRPREFIX(machine, "pc-i440") ||
src/qemu/qemu_domain.c:            STRPREFIX(machine, "rhel"));
src/qemu/qemu_domain.c-}

Need to grep for callers of this function.

---
src/qemu/qemu_domain.c=qemuDomainMachineNeedsFDC(const char *machine)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c:    const char *p = STRSKIP(machine, "pc-q35-");
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    if (p) {
src/qemu/qemu_domain.c-        if (STRPREFIX(p, "1.") ||
src/qemu/qemu_domain.c-            STRPREFIX(p, "2.0") ||
src/qemu/qemu_domain.c-            STRPREFIX(p, "2.1") ||
src/qemu/qemu_domain.c-            STRPREFIX(p, "2.2") ||
src/qemu/qemu_domain.c-            STRPREFIX(p, "2.3"))
src/qemu/qemu_domain.c-            return false;
src/qemu/qemu_domain.c-        return true;
src/qemu/qemu_domain.c-    }
src/qemu/qemu_domain.c-    return false;
src/qemu/qemu_domain.c-}

Lack of bus/defaults information on query-machines.


--
src/qemu/qemu_domain.c=qemuDomainMachineIsS390CCW(const char *machine)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c:    return STRPREFIX(machine, "s390-ccw");
src/qemu/qemu_domain.c-}

Need to grep for callers of this function.


--
src/qemu/qemu_domain.c=qemuDomainMachineIsVirt(const char *machine,
src/qemu/qemu_domain.c-                        const virArch arch)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c-    if (arch != VIR_ARCH_ARMV7L &&
src/qemu/qemu_domain.c-        arch != VIR_ARCH_AARCH64)
src/qemu/qemu_domain.c-        return false;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c:    if (STRNEQ(machine, "virt") &&
src/qemu/qemu_domain.c:        !STRPREFIX(machine, "virt-"))
src/qemu/qemu_domain.c-        return false;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    return true;
src/qemu/qemu_domain.c-}

Need to grep for callers of this function.

--
src/qemu/qemu_domain.c=qemuDomainMachineIsPSeries(const char *machine,
src/qemu/qemu_domain.c-                           const virArch arch)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c-    if (!ARCH_IS_PPC64(arch))
src/qemu/qemu_domain.c-        return false;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c:    if (STRNEQ(machine, "pseries") &&
src/qemu/qemu_domain.c:        !STRPREFIX(machine, "pseries-"))
src/qemu/qemu_domain.c-        return false;
src/qemu/qemu_domain.c-
src/qemu/qemu_domain.c-    return true;
src/qemu/qemu_domain.c-}

Need to grep for callers of this function.


--
src/qemu/qemu_domain.c=qemuDomainMachineHasBuiltinIDE(const char *machine)
src/qemu/qemu_domain.c-{
src/qemu/qemu_domain.c-    return qemuDomainMachineIsI440FX(machine) ||
src/qemu/qemu_domain.c:        STREQ(machine, "malta") ||
src/qemu/qemu_domain.c:        STREQ(machine, "sun4u") ||
src/qemu/qemu_domain.c:        STREQ(machine, "g3beige");
src/qemu/qemu_domain.c-}

Lack of bus/defaults information on query-machines.


[...]
--
src/qemu/qemu_domain_address.c=qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def,
src/qemu/qemu_domain_address.c-                                       virQEMUCapsPtr qemuCaps)
src/qemu/qemu_domain_address.c-{
src/qemu/qemu_domain_address.c-    if (def->os.arch != VIR_ARCH_ARMV7L &&
src/qemu/qemu_domain_address.c-        def->os.arch != VIR_ARCH_AARCH64)
src/qemu/qemu_domain_address.c-        return;
src/qemu/qemu_domain_address.c-
src/qemu/qemu_domain_address.c:    if (!(STRPREFIX(def->os.machine, "vexpress-") ||
src/qemu/qemu_domain_address.c-          qemuDomainIsVirt(def)))
src/qemu/qemu_domain_address.c-        return;

Lack of bus/device/defaults information on query-machines?

[...]
--
src/qemu/qemu_domain_address.c=qemuDomainSupportsPCI(virDomainDefPtr def,
src/qemu/qemu_domain_address.c-                      virQEMUCapsPtr qemuCaps)
src/qemu/qemu_domain_address.c-{
src/qemu/qemu_domain_address.c-    if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
src/qemu/qemu_domain_address.c-        return true;
src/qemu/qemu_domain_address.c-
src/qemu/qemu_domain_address.c:    if (STREQ(def->os.machine, "versatilepb"))
src/qemu/qemu_domain_address.c-        return true;
src/qemu/qemu_domain_address.c-
src/qemu/qemu_domain_address.c-    if (qemuDomainIsVirt(def) &&
src/qemu/qemu_domain_address.c-        virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX))
src/qemu/qemu_domain_address.c-        return true;
src/qemu/qemu_domain_address.c-

Lack of bus information on query-machines.

[...]

-- 
Eduardo

  reply	other threads:[~2018-05-29 14:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17 19:23 [Qemu-devel] [PATCH v7 0/3] wakeup-from-suspend and system_wakeup changes Daniel Henrique Barboza
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target Daniel Henrique Barboza
2018-05-18  8:48   ` Markus Armbruster
2018-05-21 18:14     ` Eduardo Habkost
2018-05-21 19:46       ` Daniel Henrique Barboza
2018-05-21 20:26         ` Eduardo Habkost
2018-05-23  9:17           ` Markus Armbruster
2018-05-23 12:27             ` Eduardo Habkost
2018-05-23 14:11               ` Daniel Henrique Barboza
2018-05-23 15:53               ` Markus Armbruster
2018-05-24 18:57                 ` Eduardo Habkost
2018-05-25  6:30                   ` Markus Armbruster
2018-05-25 20:30                     ` Eduardo Habkost
2018-05-28  7:23                       ` Markus Armbruster
2018-05-29 14:55                         ` Eduardo Habkost [this message]
2018-06-19 20:29                           ` Daniel Henrique Barboza
2018-06-20  7:09                             ` Markus Armbruster
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 2/3] qga: update guest-suspend-ram and guest-suspend-hybrid descriptions Daniel Henrique Barboza
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 3/3] qmp.c: system_wakeup: runstate and wake-up support check Daniel Henrique Barboza
2018-05-18  8:48   ` Markus Armbruster
2018-05-18 12:52     ` Daniel Henrique Barboza
2018-05-18 15:00       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180529145545.GB8988@localhost.localdomain \
    --to=ehabkost@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=armbru@redhat.com \
    --cc=danielhb@linux.ibm.com \
    --cc=dgilbert@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.