From: Liao Pingfang <liao.pingfang@zte.com.cn>
According to the context, here we are checking net->downscript's validity,
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn>
---
src/qemu/qemu_hotplug.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d1a2be1..8c99dfc 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4672,9 +4672,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virDomainNetReleaseActualDevice(conn, vm->def, net);
else
VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname));
- } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET) {
- if (net->script)
- virNetDevRunEthernetScript(net->ifname, net->downscript);
+ } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) {
+ virNetDevRunEthernetScript(net->ifname, net->downscript);
}
virDomainNetDefFree(net);
return 0;
--
2.9.5
On 5/29/20 5:52 PM, Yi Wang wrote: > From: Liao Pingfang <liao.pingfang@zte.com.cn> > > According to the context, here we are checking net->downscript's validity, > > Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> > --- > src/qemu/qemu_hotplug.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index d1a2be1..8c99dfc 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -4672,9 +4672,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, > virDomainNetReleaseActualDevice(conn, vm->def, net); > else > VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); > - } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET) { > - if (net->script) > - virNetDevRunEthernetScript(net->ifname, net->downscript); > + } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript) { > + virNetDevRunEthernetScript(net->ifname, net->downscript); > } > virDomainNetDefFree(net); > return 0; > I'm not quite sure why this change is needed and the commit message doesn't explain it well. My idea, when merging the original patch, was to have the following pattern: if (net->type == VIR_DOMAIN_NET_TYPE_NET) { } else if (net->type == VIR_DOMAAIN_NET_TYPE_ETHERNET) { } else if (net->type == ...) { } because it's easily extensible (e.g. if another action needs to be taken for say ethernet type interface then all that's needed is to call a function from corresponding if(). If your patch is merged then it needs to be (effectively) reverted. Can you elaborate on this please? Michal
On 6/1/20 3:51 PM, Michal Privoznik wrote: > On 5/29/20 5:52 PM, Yi Wang wrote: >> From: Liao Pingfang <liao.pingfang@zte.com.cn> >> >> According to the context, here we are checking net->downscript's >> validity, >> >> Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> >> --- >> src/qemu/qemu_hotplug.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> > > Can you elaborate on this please? > Ah, how could I miss the important change from net->script to net->downscript is beyond me :-) I've fixed the patch to follow the pattern (i.e. not join the two conditions into one), and pushed. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Sorry for the noise. Michal
© 2016 - 2024 Red Hat, Inc.