Re: [Qemu-devel] [PATCH v6 1/4] 9p: Treat multiple devices on one export as an error

[Christian Schoenebeck]

On Donnerstag, 29. August 2019 18:27:30 CEST Greg Kurz wrote:
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 586a6dccba..8cc65c2c67 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -572,10 +572,18 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
> > 
> >                                  P9_STAT_MODE_SOCKET)
> >  
> >  /* This is the algorithm from ufs in spfs */
> > 
> > -static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
> > +static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID
> > *qidp)> 
> >  {
> >  
> >      size_t size;
> > 
> > +    if (pdu->s->dev_id != stbuf->st_dev) {
> > +        error_report_once(
> > +            "9p: Multiple devices detected in same VirtFS export. "
> > +            "You must use a separate export for each device."
> > +        );
> > +        return -ENODEV;
> 
> As explained in the v5 review, we don't necessarily want to break existing
> cross-device setups that just happen to work. Moreover, the next patch
> re-allows them since remap_inodes=ignore is the default. I would thus
> only do:
> 
>         warn_report_once(
>                     "9p: Multiple devices detected in same VirtFS export, "
>                     "which might lead to file ID collisions and severe "
>                     "misbehaviours on guest! You should use a separate "
>                     "export for each device shared from host."
>         );
> 
> So I've just changed that and applied to 9p-next since it is
> a valuable improvement. Note that I've kept the signature change
> of stat_to_qid() for simplicity even if it isn't needed before
> the next patch.

I'm fine with your changes.

Dropping "return -ENODEV" in this patch was beyond my default level for 
details since this is really just a very detailed sort of git history 
tweaking. :) Like you already pointed out, not aborting (as default behaviour) 
would have been addressed with the subsequent patch anyway.