[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 07/10] qdev: Eliminate "global not used" warn

From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH v2 07/10] qdev: Eliminate "global not used" warning
Date: Thu, 23 Jun 2016 09:10:31 -0300
User-agent: Mutt/1.6.1 (2016-04-27)

On Thu, Jun 23, 2016 at 10:17:55AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> > qdev_prop_check_globals() tries to warn the user if a given
> > -global option was not used. But it does that only if the device
> > is not hotpluggable.
> Because then the option will never be used.
> > The warning also makes it harder for management code or people
> > that write their own scripts or config files: there's no way to
> > know if a given -global option will trigger a warning or not,
> > because we don't have any introspection mechanism to check if a
> > machine-type instantiates a given device or not.
> Well, there is: create the machine, look for the device.  I guess what
> you're trying to say is that there's no way to *predict* what a machine
> type will instantiate, at least not until after the fact.
> Suggest "will instantiate a given device".

I would fix it, but I will drop the patch from the series instead
(see below).

> > The warning is also the only reason we have the 'user_provided'
> > and 'used' fields in struct GlobalProperty. Removing the check
> > will let us simplify the code.
> >
> > In other words, the warning is nearly useless and gets in our way
> > and our users' way. Remove it.
> Well, telling me that I gave a non-sensical option isn't always useless.
> I might have fat-fingered something, or be confused about my devices.
> Say I give -global PIIX4_PM.disable_s3=1.  Does what I want with an
> i440FX machine type.  Except it does nothing with -no-acpi or with a Q35
> machine type.  The former is harmless enough, but the latter is a
> mistake; I need to say -global ICH9-LPC.disable_s3=1.  Before this
> patch, I get a warning in either case.
> Is this warning really useless?

Not entirely useless. But I was giving more importance to code
simplicity than to the detection of those mistakes.

But the extra fields and extra code don't bother me so much. I
will drop patches 07/10 and 08/10 from the series.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]