On Mon, May 17, 2010 at 12:10:00PM -0600, Jim Fehlig wrote:
> The qemu driver contains a subtle race in the logic to find next
> available vnc port. Currently it iterates through all available ports
> and returns the first for which bind(2) succeeds. However it is possible
> that a previously issued port has not yet been bound by qemu, resulting
> in the same port used for a subsequent domain.
>
> This patch addresses the race by using a simple bitmap to "reserve" the
> ports allocated by libvirt.
> ---
> src/qemu/qemu_driver.c | 22 ++++++++++++++++++++++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 582fdee..329859e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2629,13 +2629,24 @@ qemuInitPCIAddresses(struct qemud_driver *driver,
> return ret;
> }
>
> +static virBitmapPtr vnc_used_port_bitmap;
This should be a field in 'struct qemud_driver' rather than static.
> static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
> int i;
>
> + if (vnc_used_port_bitmap == NULL) {
> + if ((vnc_used_port_bitmap = virBitmapAlloc(59635)) == NULL)
> + return -1;
> + }
> +
> for (i = 5900 ; i < 65535 ; i++) {
> int fd;
> int reuse = 1;
> struct sockaddr_in addr;
> +
> + if (virBitmapGetBit(vnc_used_port_bitmap, i - 5900))
> + continue;
> +
> addr.sin_family = AF_INET;
> addr.sin_port = htons(i);
> addr.sin_addr.s_addr = htonl(INADDR_ANY);
> @@ -2651,6 +2662,8 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
> if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) {
> /* Not in use, lets grab it */
> close(fd);
> + /* Add port to bitmap of reserved ports */
> + virBitmapSetBit(vnc_used_port_bitmap, i - 5900);
> return i;
> }
> close(fd);
> @@ -3717,6 +3730,15 @@ retry:
>
> qemudRemoveDomainStatus(driver, vm);
>
> + /* Remove VNC port from vnc_used_port_bitmap, but only if it was reserved
> + by the driver (autoport=yes)
> + */
> + if ((vm->def->ngraphics == 1) &&
> + vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> + vm->def->graphics[0]->data.vnc.autoport)
> + virBitmapClearBit(vnc_used_port_bitmap,
> + vm->def->graphics[0]->data.vnc.port - 5900);
> +
> vm->pid = -1;
> vm->def->id = -1;
> vm->state = VIR_DOMAIN_SHUTOFF;
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list