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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxx>
