Message ID | 2c9618e155b29b6fb811de112fc03d9585c85070.1667136717.git.huangy81@chinatelecom.cn |
---|---|
State | New |
Headers | show |
Series | Fix the virtio features negotiation flaw | expand |
On Sun, Oct 30, 2022 at 09:52:38PM +0800, huangy81@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> > > Abstract vhost acked features saving into > vhost_user_save_acked_features, export it as util function. > > Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> > Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn> > --- > include/net/vhost-user.h | 2 ++ > net/vhost-user.c | 29 ++++++++++++++++++----------- > 2 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h > index 5bcd8a6..00d4661 100644 > --- a/include/net/vhost-user.h > +++ b/include/net/vhost-user.h > @@ -14,5 +14,7 @@ > struct vhost_net; > struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc); > uint64_t vhost_user_get_acked_features(NetClientState *nc); > +void vhost_user_save_acked_features(NetClientState *nc, > + bool cleanup); > > #endif /* VHOST_USER_H */ > diff --git a/net/vhost-user.c b/net/vhost-user.c > index b1a0247..74f349c 100644 > --- a/net/vhost-user.c > +++ b/net/vhost-user.c > @@ -45,24 +45,31 @@ uint64_t vhost_user_get_acked_features(NetClientState *nc) > return s->acked_features; > } > > -static void vhost_user_stop(int queues, NetClientState *ncs[]) > +void vhost_user_save_acked_features(NetClientState *nc, bool cleanup) > { > NetVhostUserState *s; > + > + s = DO_UPCAST(NetVhostUserState, nc, nc); > + if (s->vhost_net) { > + uint64_t features = vhost_net_get_acked_features(s->vhost_net); > + if (features) { > + s->acked_features = features; > + } > + > + if (cleanup) { > + vhost_net_cleanup(s->vhost_net); > + } > + } > +} I can't figure out what is going on here. Why is there a cleanup flag? What does cleanup have to do with saving acked features? I suspect it's better to just leave this part in the caller. > + > +static void vhost_user_stop(int queues, NetClientState *ncs[]) > +{ > int i; > > for (i = 0; i < queues; i++) { > assert(ncs[i]->info->type == NET_CLIENT_DRIVER_VHOST_USER); > > - s = DO_UPCAST(NetVhostUserState, nc, ncs[i]); > - > - if (s->vhost_net) { > - /* save acked features */ > - uint64_t features = vhost_net_get_acked_features(s->vhost_net); > - if (features) { > - s->acked_features = features; > - } > - vhost_net_cleanup(s->vhost_net); > - } > + vhost_user_save_acked_features(ncs[i], true); > } > } > > -- > 1.8.3.1
在 2022/11/11 2:56, Michael S. Tsirkin 写道: > On Sun, Oct 30, 2022 at 09:52:38PM +0800, huangy81@chinatelecom.cn wrote: >> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >> >> Abstract vhost acked features saving into >> vhost_user_save_acked_features, export it as util function. >> >> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >> Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn> >> --- >> include/net/vhost-user.h | 2 ++ >> net/vhost-user.c | 29 ++++++++++++++++++----------- >> 2 files changed, 20 insertions(+), 11 deletions(-) >> >> diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h >> index 5bcd8a6..00d4661 100644 >> --- a/include/net/vhost-user.h >> +++ b/include/net/vhost-user.h >> @@ -14,5 +14,7 @@ >> struct vhost_net; >> struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc); >> uint64_t vhost_user_get_acked_features(NetClientState *nc); >> +void vhost_user_save_acked_features(NetClientState *nc, >> + bool cleanup); >> >> #endif /* VHOST_USER_H */ >> diff --git a/net/vhost-user.c b/net/vhost-user.c >> index b1a0247..74f349c 100644 >> --- a/net/vhost-user.c >> +++ b/net/vhost-user.c >> @@ -45,24 +45,31 @@ uint64_t vhost_user_get_acked_features(NetClientState *nc) >> return s->acked_features; >> } >> >> -static void vhost_user_stop(int queues, NetClientState *ncs[]) >> +void vhost_user_save_acked_features(NetClientState *nc, bool cleanup) >> { >> NetVhostUserState *s; >> + >> + s = DO_UPCAST(NetVhostUserState, nc, nc); >> + if (s->vhost_net) { >> + uint64_t features = vhost_net_get_acked_features(s->vhost_net); >> + if (features) { >> + s->acked_features = features; >> + } >> + >> + if (cleanup) { >> + vhost_net_cleanup(s->vhost_net); >> + } >> + } >> +} > > I can't figure out what is going on here. Why is there a cleanup flag? > What does cleanup have to do with saving acked features? > I suspect it's better to just leave this part in the caller. > Indeed, i'll adjust the logic next version. >> + >> +static void vhost_user_stop(int queues, NetClientState *ncs[]) >> +{ >> int i; >> >> for (i = 0; i < queues; i++) { >> assert(ncs[i]->info->type == NET_CLIENT_DRIVER_VHOST_USER); >> >> - s = DO_UPCAST(NetVhostUserState, nc, ncs[i]); >> - >> - if (s->vhost_net) { >> - /* save acked features */ >> - uint64_t features = vhost_net_get_acked_features(s->vhost_net); >> - if (features) { >> - s->acked_features = features; >> - } >> - vhost_net_cleanup(s->vhost_net); >> - } >> + vhost_user_save_acked_features(ncs[i], true); >> } >> } >> >> -- >> 1.8.3.1 >
diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h index 5bcd8a6..00d4661 100644 --- a/include/net/vhost-user.h +++ b/include/net/vhost-user.h @@ -14,5 +14,7 @@ struct vhost_net; struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc); uint64_t vhost_user_get_acked_features(NetClientState *nc); +void vhost_user_save_acked_features(NetClientState *nc, + bool cleanup); #endif /* VHOST_USER_H */ diff --git a/net/vhost-user.c b/net/vhost-user.c index b1a0247..74f349c 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -45,24 +45,31 @@ uint64_t vhost_user_get_acked_features(NetClientState *nc) return s->acked_features; } -static void vhost_user_stop(int queues, NetClientState *ncs[]) +void vhost_user_save_acked_features(NetClientState *nc, bool cleanup) { NetVhostUserState *s; + + s = DO_UPCAST(NetVhostUserState, nc, nc); + if (s->vhost_net) { + uint64_t features = vhost_net_get_acked_features(s->vhost_net); + if (features) { + s->acked_features = features; + } + + if (cleanup) { + vhost_net_cleanup(s->vhost_net); + } + } +} + +static void vhost_user_stop(int queues, NetClientState *ncs[]) +{ int i; for (i = 0; i < queues; i++) { assert(ncs[i]->info->type == NET_CLIENT_DRIVER_VHOST_USER); - s = DO_UPCAST(NetVhostUserState, nc, ncs[i]); - - if (s->vhost_net) { - /* save acked features */ - uint64_t features = vhost_net_get_acked_features(s->vhost_net); - if (features) { - s->acked_features = features; - } - vhost_net_cleanup(s->vhost_net); - } + vhost_user_save_acked_features(ncs[i], true); } }