On Fri, May 25, 2018 at 08:30:59AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@xxxxxxxxxx> writes:
> > On Wed, May 23, 2018 at 05:53:34PM +0200, Markus Armbruster wrote:
[...]
> >> >> Worse, a machine type property that is static for all machine types now
> >> >> could conceivably become dynamic when we add a machine type
> >> >> configuration knob.
> >> >>
> >> >
> >> > This isn't the first time a machine capability that seems static
> >> > actually depends on other configuration arguments. We will
> >> > probably need to address this eventually.
> >>
> >> Then the best time to address it is now, provided we can :)
> >
> > I'm not sure this is the best time. If libvirt only needs the
> > runtime value and don't need any info at query-machines time, I
> > think support for this on query-machines will be left unused and
> > they will only use the query-current-machine value.
> >
> > Just giving libvirt the runtime data it wants
> > (query-current-machine) seems way better than requiring libvirt
> > to interpret a set of rules and independently calculate something
> > QEMU already knows.
>
> I wouldn't mind adding a query-current-machine to report dynamic machine
> capabilities if that helps QMP clients. query-machines could continue
> to report static machine capabilities then.
>
> However, we do need a plan on how to distribute machine capabilities
> between query-machines and query-current-machine, in particular how to
> handle changing staticness.
Handling dynamic data that becomes static is easy: we can keep it
on both.
>
> wakeup-suspend-support is static for most machine types, but dynamic for
> some. Where should it go?
I think it obviously should go on query-current-machine. Maybe
it can be added to query-machines in the future if it's deemed
useful.
>
> It needs to go into query-current-machine when its dynamic with the
> current machine. It may go there just to keep things regular even if
> its static with the current machine.
>
> Does it go into query-machines, too? If not, clients lose the ability
> to examine all machines efficiently. Even if this isn't an issue for
> wakeup-suspend-support: are we sure this can't be an issue for any
> future capabilities?
I don't think this will be an issue for wakup-suspend-support,
but this is already an issue for existing capabilities. See
below[1].
>
> If it goes into query-machines, what should its value be for the machine
> types where it's dynamic? Should it be absent, perhaps, letting clients
> know they have to consult query-current-machine to find the value?
>
> What if a capability previously thought static becomes dynamic? We can
> add it to query-current-machine just fine, but removing it from
> query-machines would be a compatibility break. Making it optional,
> too. Should all new members of MachineInfo be optional, just in case?
>
The above are questions we must ponder if we are considering
extending query-machines. We have been avoiding them for a few
years, by simply not extending query-machines very often and
letting libvirt hardcode machine-type info. :(
> These are design questions we need to ponder *now*. Picking a solution
> that satisfies current needs while ignoring future needs has bitten us
> in the posterior time and again. We're not going to successfully
> predict *all* future needs, but not even trying should be easy to beat.
>
> That's what I meant by "the best time to address it is now".
>
I agree. I just think there are countless other use cases that
require extending query-machines today. I'd prefer to use one of
them as a starting point for this design exercise, instead of
wakeup-suspend-support.
[1] Doing a:
$ git grep 'STR.*machine, "'
on libvirt source is enough to find some code demonstrating where
query-machines is already lacking today:
src/libxl/libxl_capabilities.c:583: if (STREQ(machine, "xenpv"))
src/libxl/libxl_capabilities.c:729: if (STREQ(domCaps->machine, "xenfv"))
src/libxl/libxl_driver.c:6379: if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) {
src/qemu/qemu_capabilities.c:2435: STREQ(def->os.machine, "ppce500"))
src/qemu/qemu_capabilities.c:2439: STREQ(def->os.machine, "prep"))
src/qemu/qemu_capabilities.c:2443: STREQ(def->os.machine, "bamboo"))
src/qemu/qemu_capabilities.c:2446: if (STREQ(def->os.machine, "mpc8544ds"))
src/qemu/qemu_capabilities.c:5376: STREQ(def->os.machine, "isapc");
src/qemu/qemu_command.c:3829: if (STRPREFIX(def->os.machine, "s390-virtio") &&
src/qemu/qemu_domain.c:2798: if (STREQ(def->os.machine, "isapc")) {
src/qemu/qemu_domain.c:5009: if (STREQ(def->os.machine, "versatilepb"))
src/qemu/qemu_domain.c:8222: return (STRPREFIX(machine, "pc-q35") ||
src/qemu/qemu_domain.c:8223: STREQ(machine, "q35"));
src/qemu/qemu_domain.c:8237: return (STREQ(machine, "pc") ||
src/qemu/qemu_domain.c:8238: STRPREFIX(machine, "pc-0.") ||
src/qemu/qemu_domain.c:8239: STRPREFIX(machine, "pc-1.") ||
src/qemu/qemu_domain.c:8240: STRPREFIX(machine, "pc-i440") ||
src/qemu/qemu_domain.c:8241: STRPREFIX(machine, "rhel"));
src/qemu/qemu_domain.c:8285: const char *p = STRSKIP(machine, "pc-q35-");
src/qemu/qemu_domain.c:8310: return STRPREFIX(machine, "s390-ccw");
src/qemu/qemu_domain.c:8329: if (STRNEQ(machine, "virt") &&
src/qemu/qemu_domain.c:8330: !STRPREFIX(machine, "virt-"))
src/qemu/qemu_domain.c:8351: if (STRNEQ(machine, "pseries") &&
src/qemu/qemu_domain.c:8352: !STRPREFIX(machine, "pseries-"))
src/qemu/qemu_domain.c:8564: STREQ(machine, "malta") ||
src/qemu/qemu_domain.c:8565: STREQ(machine, "sun4u") ||
src/qemu/qemu_domain.c:8566: STREQ(machine, "g3beige");
src/qemu/qemu_domain_address.c:467: if (!(STRPREFIX(def->os.machine, "vexpress-") ||
src/qemu/qemu_domain_address.c:2145: if (STREQ(def->os.machine, "versatilepb"))
src/util/virarch.c:170: } else if (STREQ(ut.machine, "amd64")) {
--
Eduardo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
