Eduardo Habkost <ehabkost@xxxxxxxxxx> writes: > On Wed, May 23, 2018 at 05:53:34PM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@xxxxxxxxxx> writes: >> >> > On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote: >> >> Eduardo Habkost <ehabkost@xxxxxxxxxx> writes: >> >> > On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote: >> > [...] >> >> >> Since no objection was made back then, this logic was put into query-target >> >> >> starting >> >> >> in v2. Still, I don't have any favorites though: query-target looks ok, >> >> >> query-machine >> >> >> looks ok and a new API looks ok too. It's all about what makes (more) sense >> >> >> in the >> >> >> management level, I think. >> >> > >> >> > I understand the original objection from Eric: having to add a >> >> > new command for every runtime flag we want to expose to the user >> >> > looks wrong to me. >> >> >> >> Agreed. >> >> >> >> > However, extending query-machines and query-target looks wrong >> >> > too, however. query-target looks wrong because this not a >> >> > property of the target. query-machines is wrong because this is >> >> > not a static property of the machine-type, but of the running >> >> > machine instance. >> >> >> >> Of the two, query-machines looks less wrong. >> >> >> >> Arguably, -no-acpi should not exist. It's an ad hoc flag that sneakily >> >> splits a few machine types into two variants, with and without ACPI. >> >> It's silently ignored for other machine types, even APCI-capable ones. >> >> >> >> If the machine type variants with and without ACPI were separate types, >> >> wakeup-suspend-support would be a static property of the machine type. >> >> >> >> However, "separate types" probably doesn't scale: I'm afraid we'd end up >> >> with an undesirable number of machine types. Avoiding that is exactly >> >> why we have machine types with configurable options. I suspect that's >> >> how ACPI should be configured (if at all). >> >> >> >> So, should we make -no-acpi sugar for a machine type parameter? And >> >> then deprecate -no-acpi for good measure? >> > >> > I think we should. >> >> Would you like to take care of it? > > Adding to my TODO-list, I hope I will be able to do it before the > next release. Thanks! >> >> > Can we have a new query command that could be an obvious >> >> > container for simple machine capabilities that are not static? A >> >> > name like "query-machine" would be generic enough for that, I >> >> > guess. >> >> >> >> Having command names differ only in a single letter is awkward, but >> >> let's focus on things other than naming now, and use >> >> query-current-machine like a working title. >> >> >> >> query-machines is wrong because wakeup-suspend-support isn't static for >> >> some machine types. >> >> >> >> query-current-machine is also kind of wrong because >> >> wakeup-suspend-support *is* static for most machine types. >> > >> > The most appropriate solution depends a lot on how/when >> > management software needs to query this. >> >> Right. >> >> > If they only need to query it at runtime for a running VM, >> > there's no reason for us to go of our way and add complexity just >> > to make it look like static data in query-machines. >> > >> > On the other hand, if they really need to query it before >> > configuring/starting a VM, it won't be useful at all to make it >> > available only at runtime. >> > >> > Daniel, when/how exactly software would need to query the new >> > flag? >> > >> > >> >> 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. wakeup-suspend-support is static for most machine types, but dynamic for some. Where should it go? 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? 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? 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". [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
