virtiofsd: Where should it live?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
Hi,
  There's been quite a bit of discussion about where virtiofsd, our
implemenation of a virtiofs daemon, should live.  I'd like to get
this settled now, because I'd like to tidy it up for the next
qemu cycle.

For reference it's based on qemu's livhost-user+chunks of libfuse.
It can't live in libfuse because we change enough of the library
to break their ABI.  It's C, and we've got ~100 patches - which
we can split into about 3 chunks.

Some suggestions so far:
  a) In contrib
     This is my current working assumption; the main objection is it's
     a bit big and pulls in a chunk of libfuse.

  b) In a submodule

  c) Just separate

Your suggestions/ideas please.  My preference is (a).

Dave

--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Marc-André Lureau-2
Hi David

On Mon, Nov 25, 2019 at 10:50 PM Dr. David Alan Gilbert
<[hidden email]> wrote:

>
> Hi,
>   There's been quite a bit of discussion about where virtiofsd, our
> implemenation of a virtiofs daemon, should live.  I'd like to get
> this settled now, because I'd like to tidy it up for the next
> qemu cycle.
>
> For reference it's based on qemu's livhost-user+chunks of libfuse.
> It can't live in libfuse because we change enough of the library
> to break their ABI.  It's C, and we've got ~100 patches - which
> we can split into about 3 chunks.
>
> Some suggestions so far:
>   a) In contrib
>      This is my current working assumption; the main objection is it's
>      a bit big and pulls in a chunk of libfuse
>
>   b) In a submodule
>
>   c) Just separate
>
> Your suggestions/ideas please.  My preference is (a).
>


It's more about code sharing and lifecycle.

The project started in a separate repository, and the proposed patches
for qemu aren't a clean series. Reviewing it is harder than it should
be, as we have to review/accept the whole thing.

As you said, it doesn't share much with qemu, but libvhost-user (which
we could quite easily copy or make standalone/submodule).

Then it dumps code from libfuse that is questionnable (showing age)
and often redundant with facilities provided by either glib, qemu
utils etc.

Is vhost-user-fs (the qemu device) going to have a strong relation
with virtiofsd?
Are we going to support different version of qemu and virtiofsd
combination? I suppose we have to, as vhost-user protocol should allow
that, and it's nice to allow other to experiment and implement it in
different ways.
If not, then perhaps we should think about introducing some version
checking between qemu and external processes (with config_stamp,
similar to modules).

From what I understand, I think c) would be fine. However, for
convenience/testing reasons, b) would be my preference.

--
Marc-André Lureau

Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Daniel P. Berrangé
In reply to this post by Dr. David Alan Gilbert (git)
On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:

> Hi,
>   There's been quite a bit of discussion about where virtiofsd, our
> implemenation of a virtiofs daemon, should live.  I'd like to get
> this settled now, because I'd like to tidy it up for the next
> qemu cycle.
>
> For reference it's based on qemu's livhost-user+chunks of libfuse.
> It can't live in libfuse because we change enough of the library
> to break their ABI.  It's C, and we've got ~100 patches - which
> we can split into about 3 chunks.
>
> Some suggestions so far:
>   a) In contrib
>      This is my current working assumption; the main objection is it's
>      a bit big and pulls in a chunk of libfuse.

My main objection to 'contrib/' is actually the perceived notions
about what the contrib directory is for. When I see 'contrib/'
code in either QEMU, or other open source projects, my general
impression is that this is largely unsupported code which is just
there as it might be interesting to someone, and doesn't typically
get much ongoing dev attention.

Parts that are fully supported & actively developed by projects
usually live elsewhere like a src/ or lib/ or tools/ directory.

This has kind of been the case with QEMU historically, with
the vhost-user-blk, vhost-user-scsi not being real production
quality implementations. Rather they are just technology demos
to show what you might do.   vhost-user-gpu/input blurred this
boundary a bit as they're more supported tools, and so I'd
argue contrib/ probably wasn't the right place for them either
in hindsight.

virtiofsd is definitely different as it is intended to be a
fully production quality supported tool with active dev into
the future IIUC.

IOW, if we did decide we want it in QEMU, then instead of
'$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.

>   b) In a submodule
>
>   c) Just separate
>
> Your suggestions/ideas please.  My preference is (a).

What I'm wondering is just how much sharing / overlap of code and concepts
and community operation there is going otbetween QEMU and virtiofsd. From
the tech POV, IIUC, the main blocker it would need to be in QEMU is because
it links to libvhost-user and we've not declared that to be a stable API
for 3rd party linking.

Personally I'm always biased towards self-contained apps being in their
own repositories, rather than bundling too much stuff into one repo. You
can see that in the way we've created independant git repos for any libvirt
module that didn't need to be part of the main libvirt.git.

To me the key benefit this gives is flexibility in approach. ie the app
doesn't need to blindly follow every precedent that QEMU has set. It
can instead take the most appropriate path for its needs. For example...

It could use meson for its build system already. This would be good as
builds will be done in a matter of seconds. For contributors it would
be a much less daunting project to join as it wouldn't be lost in the
firehose of other non-virtiofsd contributions on qemu-devel.

It doesn't have to follow QEMU's 3-times a year release model, with 6
week long freeze periods. It can be more agile releasing 6 times a year
with 1 week freezes if desired, I personally think tihs would be quite
desirable for a young project like virtiofsd which is evolving rapidly
as it would get new work available to users much more rapidly.

It doesn't have to follow QEMU's API stability & deprecation policies.
It could be more flexible in taking non-compatible changes, which again
may be valuable for a young rapidly evolving app.



Anyway to be clear, I'm not a contributor to virtiofsd, nor likely to
be one in the future, so just consider this a personal POV. From QEMU's
POV I don't think it'll matter whether virtiofsd in or out of QEMU git.
It is more about the impact & burden QEMU's dev process & standards might
impose on virtiofsd itself.

I'm fine with whatever option above is chosen, with the only caveat
being that if its in qemu.git, I don't think it belongs under contrib/
it should be a top level dir of its own.

Regards,
Daniel
--
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
In reply to this post by Marc-André Lureau-2
* Marc-André Lureau ([hidden email]) wrote:

> Hi David
>
> On Mon, Nov 25, 2019 at 10:50 PM Dr. David Alan Gilbert
> <[hidden email]> wrote:
> >
> > Hi,
> >   There's been quite a bit of discussion about where virtiofsd, our
> > implemenation of a virtiofs daemon, should live.  I'd like to get
> > this settled now, because I'd like to tidy it up for the next
> > qemu cycle.
> >
> > For reference it's based on qemu's livhost-user+chunks of libfuse.
> > It can't live in libfuse because we change enough of the library
> > to break their ABI.  It's C, and we've got ~100 patches - which
> > we can split into about 3 chunks.
> >
> > Some suggestions so far:
> >   a) In contrib
> >      This is my current working assumption; the main objection is it's
> >      a bit big and pulls in a chunk of libfuse
> >
> >   b) In a submodule
> >
> >   c) Just separate
> >
> > Your suggestions/ideas please.  My preference is (a).
> >
>
>
> It's more about code sharing and lifecycle.
>
> The project started in a separate repository, and the proposed patches
> for qemu aren't a clean series. Reviewing it is harder than it should
> be, as we have to review/accept the whole thing.
>
> As you said, it doesn't share much with qemu, but libvhost-user (which
> we could quite easily copy or make standalone/submodule).
>
> Then it dumps code from libfuse that is questionnable (showing age)
> and often redundant with facilities provided by either glib, qemu
> utils etc.

The libfuse code is pretty much upto date.

> Is vhost-user-fs (the qemu device) going to have a strong relation
> with virtiofsd?
> Are we going to support different version of qemu and virtiofsd
> combination? I suppose we have to, as vhost-user protocol should allow
> that, and it's nice to allow other to experiment and implement it in
> different ways.
> If not, then perhaps we should think about introducing some version
> checking between qemu and external processes (with config_stamp,
> similar to modules).

It should support mismatched versions.
We do have at least two extensions over the base we're working on
(DAX and notification for blocking locks); I'd expect
the sets of these to be posted close together but not be required
to go in at the same time.

> From what I understand, I think c) would be fine. However, for
> convenience/testing reasons, b) would be my preference.

Dave

> --
> Marc-André Lureau
>
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
In reply to this post by Daniel P. Berrangé
* Daniel P. Berrangé ([hidden email]) wrote:

> On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:
> > Hi,
> >   There's been quite a bit of discussion about where virtiofsd, our
> > implemenation of a virtiofs daemon, should live.  I'd like to get
> > this settled now, because I'd like to tidy it up for the next
> > qemu cycle.
> >
> > For reference it's based on qemu's livhost-user+chunks of libfuse.
> > It can't live in libfuse because we change enough of the library
> > to break their ABI.  It's C, and we've got ~100 patches - which
> > we can split into about 3 chunks.
> >
> > Some suggestions so far:
> >   a) In contrib
> >      This is my current working assumption; the main objection is it's
> >      a bit big and pulls in a chunk of libfuse.
>
> My main objection to 'contrib/' is actually the perceived notions
> about what the contrib directory is for. When I see 'contrib/'
> code in either QEMU, or other open source projects, my general
> impression is that this is largely unsupported code which is just
> there as it might be interesting to someone, and doesn't typically
> get much ongoing dev attention.
>
> Parts that are fully supported & actively developed by projects
> usually live elsewhere like a src/ or lib/ or tools/ directory.
>
> This has kind of been the case with QEMU historically, with
> the vhost-user-blk, vhost-user-scsi not being real production
> quality implementations. Rather they are just technology demos
> to show what you might do.   vhost-user-gpu/input blurred this
> boundary a bit as they're more supported tools, and so I'd
> argue contrib/ probably wasn't the right place for them either
> in hindsight.
>
> virtiofsd is definitely different as it is intended to be a
> fully production quality supported tool with active dev into
> the future IIUC.
>
> IOW, if we did decide we want it in QEMU, then instead of
> '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.

I'm not sure it deserves a new top level for such a specific tool.

> >   b) In a submodule
> >
> >   c) Just separate
> >
> > Your suggestions/ideas please.  My preference is (a).
>
> What I'm wondering is just how much sharing / overlap of code and concepts
> and community operation there is going otbetween QEMU and virtiofsd. From
> the tech POV, IIUC, the main blocker it would need to be in QEMU is because
> it links to libvhost-user and we've not declared that to be a stable API
> for 3rd party linking.
>
> Personally I'm always biased towards self-contained apps being in their
> own repositories, rather than bundling too much stuff into one repo. You
> can see that in the way we've created independant git repos for any libvirt
> module that didn't need to be part of the main libvirt.git.
>
> To me the key benefit this gives is flexibility in approach. ie the app
> doesn't need to blindly follow every precedent that QEMU has set. It
> can instead take the most appropriate path for its needs. For example...
>
> It could use meson for its build system already. This would be good as
> builds will be done in a matter of seconds. For contributors it would
> be a much less daunting project to join as it wouldn't be lost in the
> firehose of other non-virtiofsd contributions on qemu-devel.
>
> It doesn't have to follow QEMU's 3-times a year release model, with 6
> week long freeze periods. It can be more agile releasing 6 times a year
> with 1 week freezes if desired, I personally think tihs would be quite
> desirable for a young project like virtiofsd which is evolving rapidly
> as it would get new work available to users much more rapidly.

Form virtiofsd's point of view I'm not that worried about the release
cycle;  Given that features have to go through virtio standardisation,
the release ycle is unlikely to be a bottleneck.

> It doesn't have to follow QEMU's API stability & deprecation policies.
> It could be more flexible in taking non-compatible changes, which again
> may be valuable for a young rapidly evolving app.
>
>
>
> Anyway to be clear, I'm not a contributor to virtiofsd, nor likely to
> be one in the future, so just consider this a personal POV. From QEMU's
> POV I don't think it'll matter whether virtiofsd in or out of QEMU git.
> It is more about the impact & burden QEMU's dev process & standards might
> impose on virtiofsd itself.

As a qemu contributor, your opinion is welcome!  No need to sit on the
fence.

> I'm fine with whatever option above is chosen, with the only caveat
> being that if its in qemu.git, I don't think it belongs under contrib/
> it should be a top level dir of its own.
>

Dave

> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Michael S. Tsirkin-4
In reply to this post by Dr. David Alan Gilbert (git)
On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:
> Hi,
>   There's been quite a bit of discussion about where virtiofsd, our
> implemenation of a virtiofs daemon, should live.  I'd like to get
> this settled now, because I'd like to tidy it up for the next
> qemu cycle.
>
> For reference it's based on qemu's livhost-user+chunks of libfuse.
> It can't live in libfuse because we change enough of the library
> to break their ABI.

Generally there could be some ifdefs that allow one to
build libfuse-host or whatever from the same source.
I am guessing the big reason this doesn't fly is that
libfuse is not actively developed anymore.

Given that, the main remaining part is libvhost-user,
and it's less work to use than to duplicate that.
That kind of dictates being in qemu.

>  It's C, and we've got ~100 patches - which
> we can split into about 3 chunks.
>
> Some suggestions so far:
>   a) In contrib
>      This is my current working assumption; the main objection is it's
>      a bit big and pulls in a chunk of libfuse.
>   b) In a submodule
>
>   c) Just separate
>
> Your suggestions/ideas please.  My preference is (a).
>
> Dave


My preference is close to a, and maybe to avoid confusion we should have
a new top-level directory for "separate daemons qemu invokes, and need
to be built together with qemu". libvhost-user would have to move there,
too. "modules"?


>
> --
> Dr. David Alan Gilbert / [hidden email] / Manchester, UK
>


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Peter Maydell-5
In reply to this post by Dr. David Alan Gilbert (git)
On Tue, 26 Nov 2019 at 12:15, Dr. David Alan Gilbert
<[hidden email]> wrote:
>
> * Daniel P. Berrangé ([hidden email]) wrote:
> > My main objection to 'contrib/' is actually the perceived notions
> > about what the contrib directory is for. When I see 'contrib/'
> > code in either QEMU, or other open source projects, my general
> > impression is that this is largely unsupported code which is just
> > there as it might be interesting to someone, and doesn't typically
> > get much ongoing dev attention.

> > virtiofsd is definitely different as it is intended to be a
> > fully production quality supported tool with active dev into
> > the future IIUC.
> >
> > IOW, if we did decide we want it in QEMU, then instead of
> > '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
>
> I'm not sure it deserves a new top level for such a specific tool.

Maybe, but I think I agree with Daniel that 'contrib/' is
probably not the right place for it if it's something that
we care about supporting. 'contrib' to me is "bucket of stuff
that we didn't really feel strongly we wanted to reject but
which is probably random special-cases or other obscure
stuff, don't bother looking in here and don't assume it's
going to work either".

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Markus Armbruster
Peter Maydell <[hidden email]> writes:

> On Tue, 26 Nov 2019 at 12:15, Dr. David Alan Gilbert
> <[hidden email]> wrote:
>>
>> * Daniel P. Berrangé ([hidden email]) wrote:
>> > My main objection to 'contrib/' is actually the perceived notions
>> > about what the contrib directory is for. When I see 'contrib/'
>> > code in either QEMU, or other open source projects, my general
>> > impression is that this is largely unsupported code which is just
>> > there as it might be interesting to someone, and doesn't typically
>> > get much ongoing dev attention.
>
>> > virtiofsd is definitely different as it is intended to be a
>> > fully production quality supported tool with active dev into
>> > the future IIUC.
>> >
>> > IOW, if we did decide we want it in QEMU, then instead of
>> > '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
>>
>> I'm not sure it deserves a new top level for such a specific tool.
>
> Maybe, but I think I agree with Daniel that 'contrib/' is
> probably not the right place for it if it's something that
> we care about supporting. 'contrib' to me is "bucket of stuff
> that we didn't really feel strongly we wanted to reject but
> which is probably random special-cases or other obscure
> stuff, don't bother looking in here and don't assume it's
> going to work either".

Agree.

We have source for several separate programs in the root directory
already: qemu-bridge-helper, qemu-edid, qemu-img, qemu-io, qemu-nbd,
qemu-keymap, qemu-seccomp, qemu-ga.  Just a .c file when that suffixes,
else a subdirectory, except for qemu-io, which is two .c files in the
root, plus include/qemu-io.h.  Putting virtiofsd/ there follows
qemu-ga's precedence.

There's also precedence for putting such programs into their subsystem's
sub-directory: fsdev/virtfs-proxy-helper, scsi/pr-manager-helper.


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Thomas Huth-3
On 02/12/2019 13.56, Markus Armbruster wrote:

> Peter Maydell <[hidden email]> writes:
>
>> On Tue, 26 Nov 2019 at 12:15, Dr. David Alan Gilbert
>> <[hidden email]> wrote:
>>>
>>> * Daniel P. Berrangé ([hidden email]) wrote:
>>>> My main objection to 'contrib/' is actually the perceived notions
>>>> about what the contrib directory is for. When I see 'contrib/'
>>>> code in either QEMU, or other open source projects, my general
>>>> impression is that this is largely unsupported code which is just
>>>> there as it might be interesting to someone, and doesn't typically
>>>> get much ongoing dev attention.
>>
>>>> virtiofsd is definitely different as it is intended to be a
>>>> fully production quality supported tool with active dev into
>>>> the future IIUC.
>>>>
>>>> IOW, if we did decide we want it in QEMU, then instead of
>>>> '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
>>>
>>> I'm not sure it deserves a new top level for such a specific tool.
>>
>> Maybe, but I think I agree with Daniel that 'contrib/' is
>> probably not the right place for it if it's something that
>> we care about supporting. 'contrib' to me is "bucket of stuff
>> that we didn't really feel strongly we wanted to reject but
>> which is probably random special-cases or other obscure
>> stuff, don't bother looking in here and don't assume it's
>> going to work either".
>
> Agree.
>
> We have source for several separate programs in the root directory
> already: qemu-bridge-helper, qemu-edid, qemu-img, qemu-io, qemu-nbd,
> qemu-keymap, qemu-seccomp, qemu-ga.  Just a .c file when that suffixes,
> else a subdirectory, except for qemu-io, which is two .c files in the
> root, plus include/qemu-io.h.  Putting virtiofsd/ there follows
> qemu-ga's precedence.

IMHO the root directory is still way too overcrowded. Maybe we should
simply introduce a "tools" subdirectory?

 Thomas


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Markus Armbruster
Thomas Huth <[hidden email]> writes:

> On 02/12/2019 13.56, Markus Armbruster wrote:
>> Peter Maydell <[hidden email]> writes:
>>
>>> On Tue, 26 Nov 2019 at 12:15, Dr. David Alan Gilbert
>>> <[hidden email]> wrote:
>>>>
>>>> * Daniel P. Berrangé ([hidden email]) wrote:
>>>>> My main objection to 'contrib/' is actually the perceived notions
>>>>> about what the contrib directory is for. When I see 'contrib/'
>>>>> code in either QEMU, or other open source projects, my general
>>>>> impression is that this is largely unsupported code which is just
>>>>> there as it might be interesting to someone, and doesn't typically
>>>>> get much ongoing dev attention.
>>>
>>>>> virtiofsd is definitely different as it is intended to be a
>>>>> fully production quality supported tool with active dev into
>>>>> the future IIUC.
>>>>>
>>>>> IOW, if we did decide we want it in QEMU, then instead of
>>>>> '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
>>>>
>>>> I'm not sure it deserves a new top level for such a specific tool.
>>>
>>> Maybe, but I think I agree with Daniel that 'contrib/' is
>>> probably not the right place for it if it's something that
>>> we care about supporting. 'contrib' to me is "bucket of stuff
>>> that we didn't really feel strongly we wanted to reject but
>>> which is probably random special-cases or other obscure
>>> stuff, don't bother looking in here and don't assume it's
>>> going to work either".
>>
>> Agree.
>>
>> We have source for several separate programs in the root directory
>> already: qemu-bridge-helper, qemu-edid, qemu-img, qemu-io, qemu-nbd,
>> qemu-keymap, qemu-seccomp, qemu-ga.  Just a .c file when that suffixes,
>> else a subdirectory, except for qemu-io, which is two .c files in the
>> root, plus include/qemu-io.h.  Putting virtiofsd/ there follows
>> qemu-ga's precedence.
>
> IMHO the root directory is still way too overcrowded. Maybe we should
> simply introduce a "tools" subdirectory?

Maybe.  In general, I prefer my source trees shallow.

We've sucked at keeping new files out of the root that don't belong
there.  Mending our ways going forward is just one half of the fix.  The
other half is cleaning up the mess we made.

The manual should be somewhere below docs/.

Several .[ch] should be in a suitable subdirectory.

    $ git-ls-files | grep -v / | grep '\.[ch]$'
    arch_init.c
    balloon.c
    block.c
    blockdev-nbd.c
    blockdev.c
    blockjob.c
    bootdevice.c
    bt-host.c
    bt-vhci.c
    cpus-common.c
    cpus.c
    device-hotplug.c
    device_tree.c
    disas.c
    dma-helpers.c
    exec-vary.c
    exec.c
    gdbstub.c
    ioport.c
    iothread.c
    job-qmp.c
    job.c
    memory.c
    memory_ldst.inc.c
    memory_mapping.c
    module-common.c
    os-posix.c
    os-win32.c
    qdev-monitor.c
    qemu-bridge-helper.c
    qemu-edid.c
    qemu-img.c
    qemu-io-cmds.c
    qemu-io.c
    qemu-keymap.c
    qemu-nbd.c
    qemu-options-wrapper.h
    qemu-options.h
    qemu-seccomp.c
    qtest.c
    replication.c
    replication.h
    thunk.c
    tpm.c
    vl.c


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
* Markus Armbruster ([hidden email]) wrote:

> Thomas Huth <[hidden email]> writes:
>
> > On 02/12/2019 13.56, Markus Armbruster wrote:
> >> Peter Maydell <[hidden email]> writes:
> >>
> >>> On Tue, 26 Nov 2019 at 12:15, Dr. David Alan Gilbert
> >>> <[hidden email]> wrote:
> >>>>
> >>>> * Daniel P. Berrangé ([hidden email]) wrote:
> >>>>> My main objection to 'contrib/' is actually the perceived notions
> >>>>> about what the contrib directory is for. When I see 'contrib/'
> >>>>> code in either QEMU, or other open source projects, my general
> >>>>> impression is that this is largely unsupported code which is just
> >>>>> there as it might be interesting to someone, and doesn't typically
> >>>>> get much ongoing dev attention.
> >>>
> >>>>> virtiofsd is definitely different as it is intended to be a
> >>>>> fully production quality supported tool with active dev into
> >>>>> the future IIUC.
> >>>>>
> >>>>> IOW, if we did decide we want it in QEMU, then instead of
> >>>>> '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
> >>>>
> >>>> I'm not sure it deserves a new top level for such a specific tool.
> >>>
> >>> Maybe, but I think I agree with Daniel that 'contrib/' is
> >>> probably not the right place for it if it's something that
> >>> we care about supporting. 'contrib' to me is "bucket of stuff
> >>> that we didn't really feel strongly we wanted to reject but
> >>> which is probably random special-cases or other obscure
> >>> stuff, don't bother looking in here and don't assume it's
> >>> going to work either".
> >>
> >> Agree.
> >>
> >> We have source for several separate programs in the root directory
> >> already: qemu-bridge-helper, qemu-edid, qemu-img, qemu-io, qemu-nbd,
> >> qemu-keymap, qemu-seccomp, qemu-ga.  Just a .c file when that suffixes,
> >> else a subdirectory, except for qemu-io, which is two .c files in the
> >> root, plus include/qemu-io.h.  Putting virtiofsd/ there follows
> >> qemu-ga's precedence.
> >
> > IMHO the root directory is still way too overcrowded. Maybe we should
> > simply introduce a "tools" subdirectory?
>
> Maybe.  In general, I prefer my source trees shallow.

I think I agree with Thomas that it should be in a subdirectory for all
tools like that; creating virtiofsd at the top level feels wrong to me
since it's just too specific.  Someone please pick a name :-)

> We've sucked at keeping new files out of the root that don't belong
> there.  Mending our ways going forward is just one half of the fix.  The
> other half is cleaning up the mess we made.

It's been getting better over time mostly.
We could lose qemu-bridge-helper.c into this new directory.

Dave

> The manual should be somewhere below docs/.
>
> Several .[ch] should be in a suitable subdirectory.
>
>     $ git-ls-files | grep -v / | grep '\.[ch]$'
>     arch_init.c
>     balloon.c
>     block.c
>     blockdev-nbd.c
>     blockdev.c
>     blockjob.c
>     bootdevice.c
>     bt-host.c
>     bt-vhci.c
>     cpus-common.c
>     cpus.c
>     device-hotplug.c
>     device_tree.c
>     disas.c
>     dma-helpers.c
>     exec-vary.c
>     exec.c
>     gdbstub.c
>     ioport.c
>     iothread.c
>     job-qmp.c
>     job.c
>     memory.c
>     memory_ldst.inc.c
>     memory_mapping.c
>     module-common.c
>     os-posix.c
>     os-win32.c
>     qdev-monitor.c
>     qemu-bridge-helper.c
>     qemu-edid.c
>     qemu-img.c
>     qemu-io-cmds.c
>     qemu-io.c
>     qemu-keymap.c
>     qemu-nbd.c
>     qemu-options-wrapper.h
>     qemu-options.h
>     qemu-seccomp.c
>     qtest.c
>     replication.c
>     replication.h
>     thunk.c
>     tpm.c
>     vl.c
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
In reply to this post by Michael S. Tsirkin-4
* Michael S. Tsirkin ([hidden email]) wrote:

> On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:
> > Hi,
> >   There's been quite a bit of discussion about where virtiofsd, our
> > implemenation of a virtiofs daemon, should live.  I'd like to get
> > this settled now, because I'd like to tidy it up for the next
> > qemu cycle.
> >
> > For reference it's based on qemu's livhost-user+chunks of libfuse.
> > It can't live in libfuse because we change enough of the library
> > to break their ABI.
>
> Generally there could be some ifdefs that allow one to
> build libfuse-host or whatever from the same source.
> I am guessing the big reason this doesn't fly is that
> libfuse is not actively developed anymore.

libfuse is certainly taking patches; so it's not dead.
However, the changes for the transport are quite invasive,
and it doesn't feel right to impose them on it.
We've pushed up small fixes/changes etc - but not things
that are big intrusive lumps for our use.

> Given that, the main remaining part is libvhost-user,
> and it's less work to use than to duplicate that.
> That kind of dictates being in qemu.
>
> >  It's C, and we've got ~100 patches - which
> > we can split into about 3 chunks.
> >
> > Some suggestions so far:
> >   a) In contrib
> >      This is my current working assumption; the main objection is it's
> >      a bit big and pulls in a chunk of libfuse.
> >   b) In a submodule
> >
> >   c) Just separate
> >
> > Your suggestions/ideas please.  My preference is (a).
> >
> > Dave
>
>
> My preference is close to a, and maybe to avoid confusion we should have
> a new top-level directory for "separate daemons qemu invokes, and need
> to be built together with qemu". libvhost-user would have to move there,
> too. "modules"?

"modules" feels too close to "plugins" to my mind.

Dave

>
> >
> > --
> > Dr. David Alan Gilbert / [hidden email] / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Michael S. Tsirkin-4
On Mon, Dec 02, 2019 at 04:44:23PM +0000, Dr. David Alan Gilbert wrote:

> * Michael S. Tsirkin ([hidden email]) wrote:
> > On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:
> > > Hi,
> > >   There's been quite a bit of discussion about where virtiofsd, our
> > > implemenation of a virtiofs daemon, should live.  I'd like to get
> > > this settled now, because I'd like to tidy it up for the next
> > > qemu cycle.
> > >
> > > For reference it's based on qemu's livhost-user+chunks of libfuse.
> > > It can't live in libfuse because we change enough of the library
> > > to break their ABI.
> >
> > Generally there could be some ifdefs that allow one to
> > build libfuse-host or whatever from the same source.
> > I am guessing the big reason this doesn't fly is that
> > libfuse is not actively developed anymore.
>
> libfuse is certainly taking patches; so it's not dead.
> However, the changes for the transport are quite invasive,
> and it doesn't feel right to impose them on it.
> We've pushed up small fixes/changes etc - but not things
> that are big intrusive lumps for our use.

Maybe they will want these patches then ....  The big question would be
around security, e.g.  what if you rebase, how do you know they didn't
introduce what is a security hole for virtiofsd ...  But then, that
question remains even if you keep a separate tree.

> > Given that, the main remaining part is libvhost-user,
> > and it's less work to use than to duplicate that.
> > That kind of dictates being in qemu.
> >
> > >  It's C, and we've got ~100 patches - which
> > > we can split into about 3 chunks.
> > >
> > > Some suggestions so far:
> > >   a) In contrib
> > >      This is my current working assumption; the main objection is it's
> > >      a bit big and pulls in a chunk of libfuse.
> > >   b) In a submodule
> > >
> > >   c) Just separate
> > >
> > > Your suggestions/ideas please.  My preference is (a).
> > >
> > > Dave
> >
> >
> > My preference is close to a, and maybe to avoid confusion we should have
> > a new top-level directory for "separate daemons qemu invokes, and need
> > to be built together with qemu". libvhost-user would have to move there,
> > too. "modules"?
>
> "modules" feels too close to "plugins" to my mind.
>
> Dave

daemons?

> >
> > >
> > > --
> > > Dr. David Alan Gilbert / [hidden email] / Manchester, UK
> > >
> >
> --
> Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
* Michael S. Tsirkin ([hidden email]) wrote:

> On Mon, Dec 02, 2019 at 04:44:23PM +0000, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin ([hidden email]) wrote:
> > > On Mon, Nov 25, 2019 at 06:50:21PM +0000, Dr. David Alan Gilbert wrote:
> > > > Hi,
> > > >   There's been quite a bit of discussion about where virtiofsd, our
> > > > implemenation of a virtiofs daemon, should live.  I'd like to get
> > > > this settled now, because I'd like to tidy it up for the next
> > > > qemu cycle.
> > > >
> > > > For reference it's based on qemu's livhost-user+chunks of libfuse.
> > > > It can't live in libfuse because we change enough of the library
> > > > to break their ABI.
> > >
> > > Generally there could be some ifdefs that allow one to
> > > build libfuse-host or whatever from the same source.
> > > I am guessing the big reason this doesn't fly is that
> > > libfuse is not actively developed anymore.
> >
> > libfuse is certainly taking patches; so it's not dead.
> > However, the changes for the transport are quite invasive,
> > and it doesn't feel right to impose them on it.
> > We've pushed up small fixes/changes etc - but not things
> > that are big intrusive lumps for our use.
>
> Maybe they will want these patches then ....  The big question would be
> around security, e.g.  what if you rebase, how do you know they didn't
> introduce what is a security hole for virtiofsd ...  But then, that
> question remains even if you keep a separate tree.

It's active but slow moving; ~10 patches/month - so not too bad to
inspect.

> > > Given that, the main remaining part is libvhost-user,
> > > and it's less work to use than to duplicate that.
> > > That kind of dictates being in qemu.
> > >
> > > >  It's C, and we've got ~100 patches - which
> > > > we can split into about 3 chunks.
> > > >
> > > > Some suggestions so far:
> > > >   a) In contrib
> > > >      This is my current working assumption; the main objection is it's
> > > >      a bit big and pulls in a chunk of libfuse.
> > > >   b) In a submodule
> > > >
> > > >   c) Just separate
> > > >
> > > > Your suggestions/ideas please.  My preference is (a).
> > > >
> > > > Dave
> > >
> > >
> > > My preference is close to a, and maybe to avoid confusion we should have
> > > a new top-level directory for "separate daemons qemu invokes, and need
> > > to be built together with qemu". libvhost-user would have to move there,
> > > too. "modules"?
> >
> > "modules" feels too close to "plugins" to my mind.
> >
> > Dave
>
> daemons?

I'm OK with that.

Dave

> > >
> > > >
> > > > --
> > > > Dr. David Alan Gilbert / [hidden email] / Manchester, UK
> > > >
> > >
> > --
> > Dr. David Alan Gilbert / [hidden email] / Manchester, UK
>
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Christophe de Dinechin-3
In reply to this post by Dr. David Alan Gilbert (git)

Dr. David Alan Gilbert writes:

> Hi,
>   There's been quite a bit of discussion about where virtiofsd, our
> implemenation of a virtiofs daemon, should live.  I'd like to get
> this settled now, because I'd like to tidy it up for the next
> qemu cycle.
>
> For reference it's based on qemu's livhost-user+chunks of libfuse.
> It can't live in libfuse because we change enough of the library
> to break their ABI.  It's C, and we've got ~100 patches - which
> we can split into about 3 chunks.
>
> Some suggestions so far:
>   a) In contrib
>      This is my current working assumption; the main objection is it's
>      a bit big and pulls in a chunk of libfuse.
>
>   b) In a submodule
>
>   c) Just separate

In so far as there is much discussion of "multi-process qemu", I wonder
if it would not be time to create a "processes" subdirectory, and have
virtiofsd be the first entry there. Thomas Huth suggested "tools", but I
tend to read "tools" as things that are used during the build process.


>
> Your suggestions/ideas please.  My preference is (a).
>
> Dave


--
Cheers,
Christophe de Dinechin (IRC c3d)


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
In reply to this post by Thomas Huth-3
We seem to be coming to the conclusion something that:

  a) It should live in the qemu tree
  b) It shouldn't live under contrib
  c) We'll create a new top level, i.e. 'daemons'
  d) virtiofsd will be daemons/virtiofsd

Now, somethings I'm less clear on:
  e) What else would move into daemons?  It was suggested
    that if we've got virtiofsd in there, then we should move
    libvhost-user - which I understand, but then it's not a
    'daemons'.
    Are there any otehr daemons that should move?

  f) Should virtiofsd always be built (if the deps are installed)?

Dave

--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Peter Maydell-5
On Tue, 3 Dec 2019 at 10:53, Dr. David Alan Gilbert <[hidden email]> wrote:

>
> We seem to be coming to the conclusion something that:
>
>   a) It should live in the qemu tree
>   b) It shouldn't live under contrib
>   c) We'll create a new top level, i.e. 'daemons'
>   d) virtiofsd will be daemons/virtiofsd
>
> Now, somethings I'm less clear on:
>   e) What else would move into daemons?  It was suggested
>     that if we've got virtiofsd in there, then we should move
>     libvhost-user - which I understand, but then it's not a
>     'daemons'.
>     Are there any otehr daemons that should move?

I like the idea of a new top level directory, but I think
'daemons' is a bit too specific -- for instance it seems to
me that qemu-img would be sensible to move out of the root,
and that's not a daemon.

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Dr. David Alan Gilbert (git)
* Peter Maydell ([hidden email]) wrote:

> On Tue, 3 Dec 2019 at 10:53, Dr. David Alan Gilbert <[hidden email]> wrote:
> >
> > We seem to be coming to the conclusion something that:
> >
> >   a) It should live in the qemu tree
> >   b) It shouldn't live under contrib
> >   c) We'll create a new top level, i.e. 'daemons'
> >   d) virtiofsd will be daemons/virtiofsd
> >
> > Now, somethings I'm less clear on:
> >   e) What else would move into daemons?  It was suggested
> >     that if we've got virtiofsd in there, then we should move
> >     libvhost-user - which I understand, but then it's not a
> >     'daemons'.
> >     Are there any otehr daemons that should move?
>
> I like the idea of a new top level directory, but I think
> 'daemons' is a bit too specific -- for instance it seems to
> me that qemu-img would be sensible to move out of the root,
> and that's not a daemon.

What would your preference be?

Thomas was suggesting 'tools'.

Dave

> thanks
> -- PMM
>
--
Dr. David Alan Gilbert / [hidden email] / Manchester, UK


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Daniel P. Berrangé
In reply to this post by Peter Maydell-5
On Tue, Dec 03, 2019 at 11:06:44AM +0000, Peter Maydell wrote:

> On Tue, 3 Dec 2019 at 10:53, Dr. David Alan Gilbert <[hidden email]> wrote:
> >
> > We seem to be coming to the conclusion something that:
> >
> >   a) It should live in the qemu tree
> >   b) It shouldn't live under contrib
> >   c) We'll create a new top level, i.e. 'daemons'
> >   d) virtiofsd will be daemons/virtiofsd
> >
> > Now, somethings I'm less clear on:
> >   e) What else would move into daemons?  It was suggested
> >     that if we've got virtiofsd in there, then we should move
> >     libvhost-user - which I understand, but then it's not a
> >     'daemons'.
> >     Are there any otehr daemons that should move?
>
> I like the idea of a new top level directory, but I think
> 'daemons' is a bit too specific -- for instance it seems to
> me that qemu-img would be sensible to move out of the root,
> and that's not a daemon.

Do we really need an extra directory level ?

IIUC, the main point against having $GIT_ROOT/virtiofsd is that
the root of our repo is quite cluttered already.

Rather than trying to create a multi-level hierarchy which adds
a debate around naming, why not address the clutter by moving
*ALL* the .c/.h files out of the root so that we have a flatter
tree:

  $GITROOT
    +- qemu-system
    |   +- vl.c
    |   +- ...most other files...
    +- qemu-img
    |   +- qemu-img.c
    +- qemu-nbd
    |   +- qemu-nbd.c
    +- qemu-io
    |   +- qemu-io.c
    |   +- qemu-io-cmds.c
    +- qemu-bridge-helper
    |   ...
    +- qemu-edid
    +- qemu-keymap
    +- qga  (already exists)

Then we can add virtiofsd and other programs at the root with no big
issue.

Regards,
Daniel
--
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Reply | Threaded
Open this post in threaded view
|

Re: virtiofsd: Where should it live?

Paolo Bonzini-5
In reply to this post by Dr. David Alan Gilbert (git)
On 26/11/19 13:14, Dr. David Alan Gilbert wrote:
>> IOW, if we did decide we want it in QEMU, then instead of
>> '$GIT/contrib/virtiofsd', I'd prefer to see '$GIT/virtiofsd'.
>
> I'm not sure it deserves a new top level for such a specific tool.
>

It could be in fsdev/virtiofsd, but I agree with Daniel that at this
point the QEMU build system introduces baggage that you may not want for
virtiofsd.

Paolo


12