Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option

[Roy Vardi]

> On 12/29/2014 03:38 PM, Roy Vardi wrote:
> >
> >
> >
> >
> >
> > > -----Original Message-----
> >
> > > From: Jason Wang [mailto:address@hidden
> >
> > > Sent: Tuesday, December 23, 2014 11:13 AM
> >
> > > To: Roy Vardi
> >
> > > Cc: address@hidden; address@hidden; Noam Camus;
> >
> > > address@hidden; address@hidden; address@hidden
> >
> > > Subject: Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net
> > tap option
> >
> > >
> >
> > >
> >
> > >
> >
> > > On Tue, Dec 23, 2014 at 4:44 PM, Roy Vardi <address@hidden
> > <mailto:address@hidden>> wrote:
> >
> > > >
> >
> > > >
> >
> > > >>  -----Original Message-----
> >
> > > >>  From: Jason Wang [mailto:address@hidden
> >
> > > >>  Sent: Monday, December 22, 2014 8:33 AM
> >
> > > >>  To: Roy Vardi; address@hidden
> > > >> <mailto:address@hidden>
> >
> > > >>  Cc: address@hidden <mailto:address@hidden>;
> > address@hidden <mailto:address@hidden>;
> address@hidden
> > <mailto:address@hidden>;
> >
> > > >> Noam Camus; address@hidden <mailto:address@hidden>
> >
> > > >>  Subject: Re: [Qemu-devel] [PATCH] net: Add persistent flag to
> > > >> -net
> >
> > > >> tap option
> >
> > > >>
> >
> > > >>
> >
> > > >>  On 12/21/2014 03:48 PM, Roy Vardi wrote:
> >
> > > >>  > From: Roy Vardi <address@hidden <mailto:address@hidden>>
> >
> > > >>  >
> >
> > > >>  >     Add 'persistent' boolean flag to -net tap option.
> >
> > > >>  >     When set to off - tap interface will be released on shutdown
> >
> > > >>  >     When set to on\not specified - tap interface will remain
> >
> > > >>
> >
> > > >>  I'm interested of the user cases in the case. Usually,
> > > >> persistent
> >
> > > >> flag was used to  let privileged application to create/configure
> > > >> the
> >
> > > >> device and then it could be  used by non-privileged users. If
> > > >> qemu
> >
> > > >> has already had privilege, why need set  persistent in this case?
> >
> > > >
> >
> > > > We're running qemu as users, non-privilege...
> >
> > > > Our work flow includes:
> >
> > > > 1. Running an internal tool for opening a persistent tap interface 2.
> >
> > > > Running qemu with the tap interface we got from above Our
> > > > environment
> >
> > > > includes a lot of such qemu runs, and we want to avoid "zombie"
> > > > tap
> >
> > > > interfaces, and we achieve it with this new flag I've added.
> >
> > >
> >
> > > I get the case, thanks for the explaining. But qemu has already had
> > method to
> >
> > > solve this. Try downscript for tap, this external script can do
> > cleanup before
> >
> > > closing tap fd.
> >
> > >
> >
> > > E.g. in your case, you may need to run tunctl -d.
> >
> >
> >
> > Thanks for the reference.
> >
> > I've checked the downscript option, but found it unsuitable for us:
> > Qemu runs the downscript before closing the fd, so a script which
> > removes the interface will fail due to busy device.
> >
> 
> Right, tunctl needs another TUNSETIFF ioctl(). You may want to delete it 
> through
> ip link del link dev $1 in your qemu-ifdown.
Yes, this works, thanks!
> >
> > I've changed the order in the qemu code so that the script is called
> > after the descriptor is closed and it works for us.
> >
> >
> >
> > What do you think about the following patch:
> >
> >
> >
> 
> This change behaviour which may break existing down scripts.
> 
> Thanks
> >
> > --- a/net/tap.c
> >
> > +++ b/net/tap.c
> >
> > @@ -296,12 +296,11 @@ static void tap_cleanup(NetClientState *nc)
> >
> >      qemu_purge_queued_packets(nc);
> >
> > -    if (s->down_script[0])
> >
> > -        launch_script(s->down_script, s->down_script_arg, s->fd);
> >
> > -
> >
> >      tap_read_poll(s, false);
> >
> >      tap_write_poll(s, false);
> >
> >      close(s->fd);
> >
> > +    if (s->down_script[0])
> >
> > +        launch_script(s->down_script, s->down_script_arg, s->fd);
> >
> >      s->fd = -1;
> >
> > }
> >
> >
> >
> > ?
> >
> >
> >
> > Thanks,
> >
> > Roy Vardi
> >