[PATCH 32/37] qemuSlirpStart: Simplify parameters

Peter Krempa pkrempa at redhat.com
Fri May 13 11:34:33 UTC 2022


On Thu, May 12, 2022 at 13:37:45 -0500, Jonathon Jongsma wrote:
> On 5/10/22 10:20 AM, Peter Krempa wrote:
> > The 'driver' can be taken from the private data of 'vm' and 'slirp' can
> > be taken from private data of 'net', both of which we need anyways.
> > 
> > Additionally by checking whether slirp needs to be started inside the
> > function we don't need to do this logic in the callers.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >   src/qemu/qemu_extdevice.c |  4 +---
> >   src/qemu/qemu_hotplug.c   |  2 +-
> >   src/qemu/qemu_slirp.c     | 10 +++++++---
> >   src/qemu/qemu_slirp.h     |  4 +---
> >   4 files changed, 10 insertions(+), 10 deletions(-)
> > diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> > index c832cfc20b..e1f06573e3 100644
> > --- a/src/qemu/qemu_slirp.c
> > +++ b/src/qemu/qemu_slirp.c
> > @@ -245,12 +245,13 @@ qemuSlirpSetupCgroup(qemuSlirp *slirp,
> > 
> > 
> >   int
> > -qemuSlirpStart(qemuSlirp *slirp,
> > -               virDomainObj *vm,
> > -               virQEMUDriver *driver,
> > +qemuSlirpStart(virDomainObj *vm,
> >                  virDomainNetDef *net,
> >                  bool incoming)
> 
> Personal taste, perhaps, but the name qemuSlirpStart() implies to me that it
> is a function that acts on a qemuSlirp* object. With this change, the
> qemuSlirp object might not even exist when the function is called. I would
> personally prefer that the function be renamed to reflect this fact.
> Something like qemuDomainStartSlirp() perhaps? Up to you if you want to
> change anything.

Too bad there's no comment for this function, but I can add it to
explain what's going on.

In most cases the function name prefix tends to originate from the file
the function is in, but we are not 100% consistent in this regard

> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>


More information about the libvir-list mailing list