Re: [PATCH 0/4] Introduce a battery, AC adapter, and lid button

[Leonid Bloch]

Hi Phil,

Thanks for your feedback! Please see below.

On Wed, Jan 20, 2021 at 11:52 PM Philippe Mathieu-Daudé <f4bug@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.

> 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.