On Thu, Jan 21, 2021 at 07:38:46AM +0200, Leonid Bloch wrote: > Hi Phil, > > Thanks for your feedback! Please see below. > > On Wed, Jan 20, 2021 at 11:52 PM Philippe Mathieu-Daudé <f4...@amsat.org> > wrote: > > > > Hi Leonid, Marcel, > > > > On 1/20/21 9:54 PM, Leonid Bloch wrote: > > > This series introduces the following ACPI devices: > > > > > > * Battery > > > * AC adapter > > > * Laptop lid button > > > > > > When running QEMU on a laptop, these paravirtualized devices reflect the > > > state of these physical devices onto the guest. This functionality is > > > relevant not only for laptops, but also for any other device which has > > > e.g. > > > a battery. This even allows to insert a ``fake'' battery to the > > > guest, in a form of a file which emulates the behavior of the actual > > > battery in sysfs. A possible use case for such a ``fake'' battery can be > > > limiting the budget of VM usage to a subscriber, in a naturally-visible > > > way. > > > > Your series looks good. Now for this feature to be even more useful for > > the community, it would be better to > > > > 1/ Have a generic (kind of abstract QDev) battery model. > > Your model would be the ISA implementation. But we could add LPC, > > SPI or I2C implementations for example. > > It definitely feels that it needs to be more generic, and I thought > how to make it so, but so far it is what I came up with. I'll think > some more, but any ideas are welcome, cause nowadays I'm doing this in > my free time only. > > > 2/ Make it a model backend accepting various kind of frontends: > > - host Linux sysfs mirroring is a particular frontend implementation > > - mirroring on Windows would be another > > - any connection (TCP) to battery simulator (Octave, ...) > > Well, it does accept an arbitrary file to represent a battery, so this > covers the battery simulator, does it? As for Windows - indeed, it > would be nice to have.
Poking at sysfs from QEMU poses a bunch of issues, for example, security, migration, etc. Running timers on the host is also not nice since it causes exits from VM ... So I agree, as a starting point let's just let user control the battery level through QMP. > > Meanwhile 2/ is not available, it would be useful to have QMP commands > > to set the battery charge and state (also max capacity). > > But the battery state is determined by the physical battery, or by an > externally provided file. Do you mean introducing another source for > battery information which will be controlled by QMP commands? > As for the max capacity, as with an actual battery, the "QEMU battery" > has it set "by the manufacturer". It is not passed through from the > host, for simplicity sake, and only the percentage is passed. How will > it help if we allow to set the max capacity? It's something pretty > much transparent to the user. (But if there is a use case, of course > it can be done.) > > > Ditto QMP command to set the LID/AC adapter state. > > > > > But of course, the main purpose here is addressing the desktop users. > > > > > > This series was tested with Windows and (desktop) Linux guests, on which > > > indeed the battery icon appears in the corresponding state (full, > > > charging, discharging, time remaining to empty, etc.) and the AC adapter > > > plugging/unplugging behaves as expected. So is the laptop lid button. > > [...] > > > > In patch #2 you comment 'if a "fake" host battery is to be provided, > > a 'sysfs_path' property allows to override the default one.'. > > > > Eventually you'd provide a such fake file as example, ideally used > > by a QTest. > > Sure! I will - it's definitely a good idea. > > > Another question. If the battery is disconnected, is there an event > > propagated to the guest? > > No. I definitely need to add! Thanks! > > > Thanks for contributing these patches :) > > Thank you! > Leonid. > > > Phil.