[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