On Wed, Sep 23, 2015 at 10:12:10AM +0800, Jason Wang wrote:
>
>
> On 09/23/2015 09:57 AM, Yuanhan Liu wrote:
> > On Tue, Sep 22, 2015 at 06:14:14PM +0800, Jason Wang wrote:
> >>
> > [...]
> >>> -static void net_vhost_link_down(VhostUserState *s, bool link_down)
> >>> +static void net_vhost_link_down(int queues, NetClientState *ncs[],
> >>> + bool link_down)
> >>> {
> >>> - s->nc.link_down = link_down;
> >>> + NetClientState *nc;
> >>> + int i;
> >>>
> >>> - if (s->nc.peer) {
> >>> - s->nc.peer->link_down = link_down;
> >>> - }
> >>> + for (i = 0; i < queues; i++) {
> >>> + nc = ncs[i];
> >>>
> >>> - if (s->nc.info->link_status_changed) {
> >>> - s->nc.info->link_status_changed(&s->nc);
> >>> - }
> >>> + nc->link_down = link_down;
> >>> +
> >>> + if (nc->peer) {
> >>> + nc->peer->link_down = link_down;
> >>> + }
> >>> +
> >>> + if (nc->info->link_status_changed) {
> >>> + nc->info->link_status_changed(nc);
> >>> + }
> >>>
> >>> - if (s->nc.peer && s->nc.peer->info->link_status_changed) {
> >>> - s->nc.peer->info->link_status_changed(s->nc.peer);
> >>> + if (nc->peer && nc->peer->info->link_status_changed) {
> >>> + nc->peer->info->link_status_changed(nc->peer);
> >>> + }
> >>> }
> >>> }
> >> The semantics is a little bit difference with qmp_set_link. What's the
> >> reason for this? If there's no specific reason, probably, we could just
> >> reuse qmp_set_link() (or part of) here?
> > I did this just because I refactored the vhost_user init code, making
> > the char dev handler to initiate all ncs. Hence I turned
> > net_vhost_link_down()
> > to handle all ncs, just like what I did for vhost_user_start().
> >
> > And TBH, I'm not aware of qmp_set_link() before, and after reading the
> > code, I do think we can simply invoke it instead. And thanks for the
> > info.
> >
> >> Other looks good to me.
> > Thanks for the review. May I add your reviewed-by if I fix above issue?
> >
> > --yliu
>
> I prefer to add it myself :)
Fair enough; I meant to save your effort :-)
--yliu
