On (Mon) Nov 30 2009 [12:39:52], Rusty Russell wrote:
> On Sat, 28 Nov 2009 05:20:32 pm Amit Shah wrote:
> > @@ -196,7 +216,9 @@ static void virtcons_apply_config(struct virtio_device *dev)
> > dev->config->get(dev,
> > offsetof(struct virtio_console_config, rows),
> > &ws.ws_row, sizeof(u16));
> > - hvc_resize(port->hvc, ws);
> > + /* This is the pre-multiport style: we use control messages
> > + * these days which specify the port. So this means port 0. */
> > + hvc_resize(find_port_by_vtermno(0)->hvc, ws);
> > }
>
> We end up doing this in a couple of places; perhaps we should have something
> like:
> /*
> * Before multiple console support, we always had a single console.
> * Code paths without those features can use this.
> */
> static struct port *old_style_unique_console(void)
> {
> return find_port_by_vtermno(0);
> }
(Again) in a later patch, I rename the function to resize_console() and
call that one from the config interrupt as well as from the
hvc notifier.
> > err = -ENOMEM;
> > - port = add_port(pdrvdata.next_vtermno);
> > + port = kzalloc(sizeof *port, GFP_KERNEL);
> > if (!port)
> > goto fail;
>
> I still dislike kzalloc. I think I've said this before :)
I remember seeing it in another thread I think (or was it in a blog post
about you liking the ability to run it through valgrind?).
The init function becomes a bit longer in that case, since we'll have to
init all the known state. But I'm fine with that.
However, there's one exception: when allocating buffers, I'd prefer to
do a kzalloc() since the buffers are sent across the guest/host boundary
and we don't want to leak data in either direction.
(Just mentioning this again since it's the last comment: I'll respin the
series once you have a chance to review the other patches too.)
Also: if you think the approach is OK and give an ACK for the approach,
I can also proceed in parallel with the host bits for QEMU.
Thanks, Rusty!
Amit
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
