qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH v2 11/15] qemu-common: move scripts/qapi


From: Marc-André Lureau
Subject: Re: [PATCH v2 11/15] qemu-common: move scripts/qapi
Date: Thu, 11 Aug 2022 14:09:24 +0400



On Thu, Aug 11, 2022 at 1:05 PM Markus Armbruster <armbru@redhat.com> wrote:
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Thu, Aug 11, 2022 at 10:52 AM Markus Armbruster <armbru@redhat.com>
> wrote:
>
>> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>>
>> > Hi
>> >
>> > On Fri, Aug 5, 2022 at 12:12 PM Markus Armbruster <armbru@redhat.com>
>> wrote:
>> >
>> >> marcandre.lureau@redhat.com writes:
>> >>
>> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >> >
>> >> > This is just moving qapi-gen.py and related subdir to qemu-common, to
>> >> > ease review and proceed step by step. The following patches will move
>> >> > related necessary code, tests etc.
>> >> >
>> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >>
>> >> As moved files tend to become low-level annoyances for a long time, I'd
>> >> like to understand why you want to move them.  The commit message says
>> >> "to ease review", which I suspect isn't the real reason.  Perhaps you
>> >> explained all that elsewhere already, but I missed it.
>> >>
>> >>
>> >>
>> > The end goal is to split some projects, such as qemu-ga, to standalone
>> > meson projects/subprojects. We will be able to build them independently
>> > from the rest of QEMU, and later on perhaps handle them outside of QEMU
>> > main repository. To achieve this, I first introduce a qemu-common
>> > subproject, where qapi and common units are provided. You can check
>> > https://gitlab.com/marcandre.lureau/qemu/-/commits/qga for a sneak peek at
>> > current end result.
>>
>> I worry this move of the QAPI generator code into
>> subjprojects/common/scripts/qapi/ will be followed by a move into its
>> own subproject.
>>
>
> Do you mean: it could be moved again to another smaller subproject? not
> really, see below
>
>
>> Ignorant question: could we turn the QAPI generator into a subproject in
>> place?
>>
>
> If it's just the generator, probably the target would then be a python
> project (not meson), similar to python-qemu-qmp.
>
> But I don't see much point, since it's not really a standalone python
> module, it generates code, and that code needs most of what is in
> qemu-common (see
> https://gitlab.com/marcandre.lureau/qemu/-/tree/qga/subprojects/qemu-common).
> It's best to have it together imho. Maybe we can consider a different
> naming or to be more careful not to add stuff that is not strictly needed
> by qapi?

I had a look at subjprojects/qemu-common in your qga branch.  Contents:

* Subproject machinery

* Some common headers (glib-compat.h), but not others (qemu/osdep.h).  I
  guess it's whatever subjproject code needs.

  Is subprojects/qemu-common/include/block/nvme.h there by accident?

It's a header shared with qemu-ga (the commit message explains)
 

* Most of the QObject subsystem

  qobject/block-qdict.c is left behind.

It's qemu block specific. Not needed to move at this point, it drags other stuff iirc.


* Most of the QAPI subsystem

  Some visitors left behind: opts, forward, string input / output.  Hmm,
  only the .c, the .h are in the subjproject.  Accident?

If they are not shared with qemu-ga, I didn't move them yet. They can stay specific to qemu or specific subprojects, or we can decide to move them (if that doesn't drag too much stuff along).
 

  A bit of HMP support left behind.

Can you be more specific?
 

* Parts of util/ and include/qemu/

  Error reporting, key-value CLI, some C utilities, but not others
  (e.g. qemu/atomic.h, but not qemu/atomic128.h).  I guess it's again
  whatever subjproject code needs.
 
* Parts of the QAPI Schema subsystem

Aside: MAINTAINERS mostly not updated.


That needs fixing.
 
Your moves tear closely related code apart.  This is going to be a drag
for developers in general and maintainers in particular.

Ergonomics suffer when related code is in multiple places.  Having to
switch between directories and remember where is what will a constant
low-level pain.  Things that used to be simple & quick, like git-grep
qapi/*.c, become more involved.


It's inevitable when a project grows. QEMU is already a very large project. Over the years, we have structured the project, by moving things and splitting in subdirectories. Imho, this is actually helpful in many ways, and we got used to it easily hopefully.

Do you see fundamental reasons why qemu-ga or (qemu-img, qemu-nbd, storage-daemon, virtiofsd, vhost-user-*, *helper, ivshmem* etc) need to be tight to qemu code, release and management process? I am not saying it's time to move them out of qemu project, but I believe it's helpful to have code that is structured and can be compiled indepently.

And by having "standalone" subprojects, we can more easily evolve in new directions, without touching the rest of the projects.

Hurts even when merely consuming the subsystem: when I see #include
"qemu/foo.h", the straightforward include/qemu/foo.h may or may not do.
When it doesn't, I need to know where to look instead.

subprojects/qemu-common/include/ is a lot to type.  Sufficiently
powerful editors mitigate, but not completely.

When changes need to be applied to every instance of an abstraction,
it's easy to miss instances "elsewhere".  There's a reason the QAPI
visitors are all in one place.

I understand it's nice to have all the code in one place. At the same time, this goes against modularity and composability..
 
The actual split seems somewhat arbitrary in places.  I suspect more
code will move over time.  Invalidating "what is where" knowledge.

Are you insinuating that this is obvious today? :) By having smaller standalone projects, we have better defined scope, test range, reusability etc. And we avoid creating a jam of dependencies, making code review & change a bit easier..


I believe a serious think about other ways to accomplish your goals is
called for.

I don't claim to have the perfect solution. It's evolution, hopefully a step in a better direction. 

thanks


--
Marc-André Lureau

reply via email to

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