On Wed, Nov 13, 2013 at 05:48:28PM +0100, Ján Tomko wrote:
> On 11/13/2013 05:39 PM, Daniel P. Berrange wrote:
> > On Thu, Oct 31, 2013 at 01:07:27PM +0100, Ján Tomko wrote:
> >> Report the error in virPortAllocatorAcquire instead
> >> of doing it in every caller.
> >>
> >> The error contains the port range name instead of the intended
> >> use for the port, e.g.:
> >> Unable to find an unused port in range 'display' (65534-65535)
> >> instead of:
> >> Unable to find an unused port for SPICE
> >>
> >> This also adds error reporting when the QEMU driver could not
> >> find an unused port for VNC, VNC WebSockets or NBD migration.
> >> ---
> >> src/libxl/libxl_conf.c | 5 -----
> >> src/qemu/qemu_migration.c | 16 ++--------------
> >> src/qemu/qemu_process.c | 12 ------------
> >> src/util/virportallocator.c | 7 ++++++-
> >> tests/virportallocatortest.c | 6 ++----
> >> 5 files changed, 10 insertions(+), 36 deletions(-)
> >>
>
> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> >> index bdffdf8..daf081d 100644
> >> --- a/src/qemu/qemu_process.c
> >> +++ b/src/qemu/qemu_process.c
> >> @@ -3406,12 +3406,6 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
> >> if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
> >> goto error;
> >>
> >> - if (port == 0) {
> >> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> - _("Unable to find an unused port for SPICE"));
> >> - goto error;
> >> - }
> >> -
> >> graphics->data.spice.port = port;
> >> }
> >>
> >> @@ -3437,12 +3431,6 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
> >> if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0)
> >> goto error;
> >>
> >> - if (tlsPort == 0) {
> >> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> - _("Unable to find an unused port for SPICE TLS"));
> >> - virPortAllocatorRelease(driver->remotePorts, port);
> >> - goto error;
> >> - }
> >
> > Opps, you're loosing the release of 'port' when tlsPort allocation fails.
> > In fact there was already a broken codepath in this respect.
> >
>
> 'port' is released on the error: label. Should I move the double-release
> removal into a separate commit to make it more obvious?
Sure, sounds good.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list