All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Claudio Imbrenda" <imbrenda@linux.ibm.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	"David Hildenbrand" <david@redhat.com>,
	"Borntraeger, Christian" <borntraeger@de.ibm.com>,
	"Janosch Frank" <frankja@linux.ibm.com>,
	fiuczy@linux.ibm.com, "Halil Pasic" <pasic@linux.ibm.com>,
	nsg@linux.ibm.com, "P. Berrange, Daniel" <berrange@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH v5 1/1] util/async-teardown: wire up query-command-line-options
Date: Tue, 28 Mar 2023 09:57:52 +0200	[thread overview]
Message-ID: <87o7od2tq7.fsf@pond.sub.org> (raw)
In-Reply-To: <98a65e35-9c56-df86-66ed-f949c1fb9c96@redhat.com> (Thomas Huth's message of "Tue, 28 Mar 2023 09:20:06 +0200")

Thomas Huth <thuth@redhat.com> writes:

> On 28/03/2023 07.26, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> I am honestly not a fan of adding a more complex option,.just because
>>> query-command-line-options only returns the square holes whereas here we
>>> got a round one.
>>>
>>> Can we imagine another functionality that would be added to -teardown? If
>>> not, it's not a good design. If it works, I would add a completely dummy
>>> (no suboptions) group "async-teardown" and not modify the parsing at all.
>>
>> Does v2 implement your suggestion?
>> Message-Id: <20230320131648.61728-1-imbrenda@linux.ibm.com>
>>
>> I dislike it, because it makes query-command-line-options claim
>> -async-teardown has an option argument with unknown keys, which is
>> plainly wrong, and must be treated as a special case.  Worse, a new kind
>> of special case.
>
> I agree with Markus, it sounds like a bad idea to create a new special case for this.
>
> Paolo, what do you think of my "-run-with" suggestion here:
>
>
> https://lore.kernel.org/qemu-devel/3237c289-b8c2-6ea2-8bfb-7eeed637efc7@redhat.com/
>
> I still think that this is a good idea, even if it is a "grab-bag" as Markus said, it would give us a place where we could wire future similar options, too, without running into this problem here again and again.
>
>> Can we have a QMP command, so libvirt can use query-qmp-schema?
>
> Question is whether this could be toggled during runtime...? Or did you mean a command that just queries the setting of the option, e.g. "query-async-teardown" which then reports whether it is enabled or not?

Mildly ugly, as the command is pretty much useless except as a witness
for the CLI option.  We've done this before.

>> In case QMP becomes functional too late for the command to actually
>> work: make it always fail for now.  It can still serve as a witness for
>> -async-teardown.  If we rework QEMU startup so that QMP can do
>> everything the CLI can do, we'll make the QMP command work.
>
> Adding non-working functions sounds ugly...

Non-working functions that could totally work with QEMU startup reworked
are only mildly ugly, I think.

> Anyway, we're slowly running out of time for QEMU 8.0 ... if we can't find an easy solution, I think we should rather postpone this to the next cycle instead of rushing unfinished stuff now.

Yes.

I think we can still do it if we agree quickly on which kind of ugly we
hate the least.

Adding -async-teardown as a stable interface was premature.  We should
have marked it unstable when no libvirt patch was ready when we cut QEMU
7.2.

Adding new external interfaces is still not hard enough :)



      reply	other threads:[~2023-03-28  7:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27 13:35 [PATCH v5 0/1] util/async-teardown: appear in query-command-line-options Claudio Imbrenda
2023-03-27 13:35 ` [PATCH v5 1/1] util/async-teardown: wire up query-command-line-options Claudio Imbrenda
2023-03-27 21:16   ` Paolo Bonzini
2023-03-28  5:26     ` Markus Armbruster
2023-03-28  7:20       ` Thomas Huth
2023-03-28  7:57         ` Markus Armbruster [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o7od2tq7.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=berrange@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=david@redhat.com \
    --cc=fiuczy@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=nsg@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.