qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	qemu-devel@nongnu.org, "Eric Auger" <eric.auger@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 0/4] vl: Prioritize device realizations
Date: Thu, 21 Oct 2021 09:17:57 +0200	[thread overview]
Message-ID: <12cdd7f4-16c2-5af8-aeb2-e168506eefc2@redhat.com> (raw)
In-Reply-To: <YXDqaZL71DCEghbr@xz-m1.local>

> Yes I should follow this up, thanks for asking.
> 
> Though after Markus and Igor pointed out to me that it's much more than types
> of device and objects to order, I don't have a good way to fix the ordering
> issue for good on all the problems; obviously current solution only applies to
> device class ordering.
> 
> Examples that Markus provided:
> 
> https://lore.kernel.org/qemu-devel/87ilzj81q7.fsf@dusky.pond.sub.org/
> 
> Also there can be inter-dependency issue too for single device class, e.g., for
> pci buses if bus pcie.2 has a parent pci bus of pcie.1, then we must speficy
> the "-device" for pcie.1 before the "-device" of pcie.2, otherwise qemu will
> fail to boot too.
> 
Interesting case :)

> Any of above examples means ordering based on device class can only solve
> partial of the problems, not all.
> 
> And I can buy in with what people worry about on having yet another way to fix
> ordering, since the root issue is still unsettled, even if the current solution
> seems to work for vIOMMU/vfio, and I had a feeling it could work too with the
> virtio-mem issue you're tackling with.

The question is if we need to get it all 100% right from the start. To
me, for example, the single device class issue is a whole different beast.

I know, whenever someone proposes a way to tackle part of a challenging
problem, everybody discovers their hopes and dreams and suddenly you
have to go all the way to solve the complete problem. The end result is
that there is no improvement at all instead of incremental improvement.

> 
> My plan is to move on with what Igor suggested, on using either pre_plug hook
> for vIOMMU to make sure no special devices like vfio are realized before that.
> I think it'll be as silly as a pci bus scan on all the pcie host bridges
> looking for vfio-pci, it can even be put directly into realize() I think as I
> don't see an obvious difference on failing pre_plug() or realize() so far.
> Then I'll just drop this series so the new version may not really help with
> virtio-mem anymore; though not sure virtio-mem can do similar thing.

In case of virtio-mem, we'll already fail properly when realizing the
vhost-* device and figuring out that the memslot limit the device
imposes isn't going to work. So what you would want to have for vIOMMU
is already in place (fail instead of silently continue).

> 
> One step back, OTOH, I do second on what Daniel commented in the other thread
> on leaving that problem to the user; sad to know that we already have pmem
> restriction so hot plugging some device already start to fail, but maybe
> failing is fine as long as nothing will crash? :)

I very much agree.

There are two cases:

1. QEMU failing to start because vhost-* was specified after virtio-mem.
We get an indication that points at the number of memslots. And as the
user explicitly enabled e.g., "max-memslots=0", I think that's fair enough.

2. Hotplug of vhost-* devices failing. We similarly get an indication
that points at the number of memslots. Similarly, I think that's fair
enough. The guest will continue working just fine.

The result of that discussion is that the default should always be
"max-memslots=1" and that users have to opt in manually.

> 
> I also do think it's nice to at least allow the user to specify the exact value
> of virtio-mem slot number without any smart tricks to be played when the user
> wants - I think it's still okay to do automatic detection, but that's already
> part of "policy" not "mechanism" to me so imho it should be better optional,
> and now I had a feeling that maybe qemu should be good enough on providing
> these mechanisms first then we leave the rest of the problems to libvirt, maybe
> that's a better place to do all these sanity checks and doing smart things on
> deciding the slot numbers.  For qemu failing at the right point without
> interrupting the guest seems to be good enough so far.

I'm not planning on letting the user set the actual number of memslots
to use, only an upper limit. But to me, it's fundamentally the same: the
user has to enable this behavior explicitly.

> 
> I think "early failing" seems to not be a problem for virtio-mem already since
> if there's a conflict on the slot number then e.g. vhost-user will already fail
> early, not sure whether it means it's good enough.  For vIOMMU I may need to
> work on the other bus scanning patchset to make sure when vfio is specified
> before vIOMMU then we should fail qemu early, and that's still missing.

Right, thanks!


-- 
Thanks,

David / dhildenb



  reply	other threads:[~2021-10-21  7:19 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18 19:42 [PATCH 0/4] vl: Prioritize device realizations Peter Xu
2021-08-18 19:42 ` [PATCH 1/4] qdev-monitor: Trace qdev creation Peter Xu
2021-08-18 19:43 ` [PATCH 2/4] qemu-config: Allow in-place sorting of QemuOptsList Peter Xu
2021-08-18 19:43 ` [PATCH 3/4] qdev: Export qdev_get_device_class() Peter Xu
2021-08-18 19:43 ` [PATCH 4/4] vl: Prioritize realizations of devices Peter Xu
2021-08-23 18:49   ` Eduardo Habkost
2021-08-23 19:18     ` Peter Xu
2021-08-23 21:07       ` Eduardo Habkost
2021-08-23 21:31         ` Peter Xu
2021-08-23 21:54           ` Michael S. Tsirkin
2021-08-23 22:51             ` Peter Xu
2021-08-23 21:56           ` Eduardo Habkost
2021-08-23 23:05             ` Peter Xu
2021-08-25  9:39               ` Markus Armbruster
2021-08-25 12:28                 ` Markus Armbruster
2021-08-25 21:50                   ` Peter Xu
2021-08-26  3:50                     ` Peter Xu
2021-08-26  8:01                       ` Markus Armbruster
2021-08-26 11:36                         ` Igor Mammedov
2021-08-26 13:43                           ` Peter Xu
2021-08-30 19:02                             ` Peter Xu
2021-08-31 11:35                               ` Markus Armbruster
2021-09-02  8:26                               ` Igor Mammedov
2021-09-02 13:45                                 ` Peter Xu
2021-09-02 13:53                                   ` Daniel P. Berrangé
2021-09-02 14:21                                     ` Peter Xu
2021-09-02 14:57                                       ` Markus Armbruster
2021-09-03 15:48                                         ` Peter Xu
2021-09-02 15:06                                       ` Daniel P. Berrangé
2021-09-02 15:26                                   ` Markus Armbruster
2021-09-03 13:00                                   ` Igor Mammedov
2021-09-03 16:03                                     ` Peter Xu
2021-09-06  8:49                                       ` Igor Mammedov
2021-09-02  7:46                             ` Igor Mammedov
2021-08-26  4:57                     ` Markus Armbruster
2021-08-23 22:05       ` Michael S. Tsirkin
2021-08-23 22:36         ` Peter Xu
2021-08-24  2:52           ` Jason Wang
2021-08-24 15:50             ` Peter Xu
2021-08-25  4:23               ` Jason Wang
2021-09-06  9:22                 ` Eric Auger
2021-08-24 16:24         ` David Hildenbrand
2021-08-24 19:52           ` Peter Xu
2021-08-25  8:08             ` David Hildenbrand
2021-08-24  2:51       ` Jason Wang
2021-10-20 13:44 ` [PATCH 0/4] vl: Prioritize device realizations David Hildenbrand
2021-10-20 13:48   ` Daniel P. Berrangé
2021-10-20 13:58     ` David Hildenbrand
2021-10-21  4:20   ` Peter Xu
2021-10-21  7:17     ` David Hildenbrand [this message]
2021-10-21  8:00       ` Peter Xu
2021-10-21 16:54         ` David Hildenbrand

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=12cdd7f4-16c2-5af8-aeb2-e168506eefc2@redhat.com \
    --to=david@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).