10:20 a.m.
Ján Tomko (2):
Add exit_monitor label to qemuMigrationStartNBDServer
Report an error if port range is exhausted
src/qemu/qemu_migration.c | 25 ++++++++++++++++---------
src/qemu/qemu_process.c | 10 ++++++++++
2 files changed, 26 insertions(+), 9 deletions(-)
--
1.8.1.5
10:20 a.m.
New subject: [libvirt] [PATCH 1/2] Add exit_monitor label to qemuMigrationStartNBDServer
---
src/qemu/qemu_migration.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index cb59620..5607098 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1117,14 +1117,11 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
if (!port &&
((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) ||
(qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) {
- qemuDomainObjExitMonitor(driver, vm);
- goto cleanup;
+ goto exit_monitor;
}
- if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
- goto cleanup;
- }
+ if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0)
+ goto exit_monitor;
qemuDomainObjExitMonitor(driver, vm);
}
@@ -1136,6 +1133,10 @@ cleanup:
if (ret < 0)
virPortAllocatorRelease(driver->remotePorts, port);
return ret;
+
+exit_monitor:
+ qemuDomainObjExitMonitor(driver, vm);
+ goto cleanup;
}
/**
--
1.8.1.5
10:20 a.m.
New subject: [libvirt] [PATCH 2/2] Report an error if port range is exhausted
If the port range is full, virPortAllocatorAcquire will
return 0 and set the port to 0, leaving the error reporting
to the caller.
This didn't happen when allocating ports for QEMU's VNC (introduced
along with the virPortAllocator in dfb1022), VNC WebSockets and
NBD migration (introduced with these features in f1ad8d2 and 86d90b3)
---
src/qemu/qemu_migration.c | 14 ++++++++++----
src/qemu/qemu_process.c | 10 ++++++++++
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5607098..addc78e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1114,10 +1114,16 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
goto cleanup;
- if (!port &&
- ((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) ||
- (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) {
- goto exit_monitor;
+ if (!port) {
+ if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
+ goto exit_monitor;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find unused port for NBD
migration"));
+ goto exit_monitor;
+ }
+ if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0)
+ goto exit_monitor;
}
if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 354e079..3d9bffc 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3345,12 +3345,22 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
if (graphics->data.vnc.autoport) {
if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find an unused port for VNC"));
+ return -1;
+ }
graphics->data.vnc.port = port;
}
if (graphics->data.vnc.websocket == -1) {
if (virPortAllocatorAcquire(driver->webSocketPorts, &port) < 0)
return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find an unused port for VNC
WebSocket"));
+ return -1;
+ }
graphics->data.vnc.websocket = port;
}
--
1.8.1.5
10:24 a.m.
New subject: [libvirt] [PATCH 2/2] Report an error if port range is exhausted
On Wed, Oct 30, 2013 at 04:20:10PM +0100, Ján Tomko wrote:
If the port range is full, virPortAllocatorAcquire will
return 0 and set the port to 0, leaving the error reporting
to the caller.
This didn't happen when allocating ports for QEMU's VNC (introduced
along with the virPortAllocator in dfb1022), VNC WebSockets and
NBD migration (introduced with these features in f1ad8d2 and 86d90b3)
---
src/qemu/qemu_migration.c | 14 ++++++++++----
src/qemu/qemu_process.c | 10 ++++++++++
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5607098..addc78e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1114,10 +1114,16 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
goto cleanup;
- if (!port &&
- ((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) ||
- (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) {
- goto exit_monitor;
+ if (!port) {
+ if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
+ goto exit_monitor;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find unused port for NBD
migration"));
+ goto exit_monitor;
+ }
+ if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0)
+ goto exit_monitor;
}
if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 354e079..3d9bffc 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3345,12 +3345,22 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
if (graphics->data.vnc.autoport) {
if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find an unused port for VNC"));
+ return -1;
+ }
graphics->data.vnc.port = port;
}
if (graphics->data.vnc.websocket == -1) {
if (virPortAllocatorAcquire(driver->webSocketPorts, &port) < 0)
return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to find an unused port for VNC
WebSocket"));
+ return -1;
+ }
graphics->data.vnc.websocket = port;
}
AFAIK, all callers have to report an error message if port == 0. As we
see from these missing error reports, this is fragile. Lets just make
virPortAllocatorAcquire() return -1 if the ports are exhausted and
report an error itself. This removes the chance that the caller will
forget.
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 :|
11 a.m.
New subject: [libvirt] [PATCH 2/2] Report an error if port range is exhausted
On 10/30/2013 04:24 PM, Daniel P. Berrange wrote:
On Wed, Oct 30, 2013 at 04:20:10PM +0100, Ján Tomko wrote:
> If the port range is full, virPortAllocatorAcquire will
> return 0 and set the port to 0, leaving the error reporting
> to the caller.
>
> This didn't happen when allocating ports for QEMU's VNC (introduced
> along with the virPortAllocator in dfb1022), VNC WebSockets and
> NBD migration (introduced with these features in f1ad8d2 and 86d90b3)
> ---
> src/qemu/qemu_migration.c | 14 ++++++++++----
> src/qemu/qemu_process.c | 10 ++++++++++
> 2 files changed, 20 insertions(+), 4 deletions(-)
>
AFAIK, all callers have to report an error message if port == 0. As we
see from these missing error reports, this is fragile. Lets just make
virPortAllocatorAcquire() return -1 if the ports are exhausted and
report an error itself. This removes the chance that the caller will
forget.
It will also remove the specific error message. It might be helpful to tell
the user which port range is exhausted.
Jan
11:03 a.m.
New subject: [libvirt] [PATCH 2/2] Report an error if port range is exhausted
On Wed, Oct 30, 2013 at 05:00:40PM +0100, Ján Tomko wrote:
On 10/30/2013 04:24 PM, Daniel P. Berrange wrote:
> On Wed, Oct 30, 2013 at 04:20:10PM +0100, Ján Tomko wrote:
>> If the port range is full, virPortAllocatorAcquire will
>> return 0 and set the port to 0, leaving the error reporting
>> to the caller.
>>
>> This didn't happen when allocating ports for QEMU's VNC (introduced
>> along with the virPortAllocator in dfb1022), VNC WebSockets and
>> NBD migration (introduced with these features in f1ad8d2 and 86d90b3)
>> ---
>> src/qemu/qemu_migration.c | 14 ++++++++++----
>> src/qemu/qemu_process.c | 10 ++++++++++
>> 2 files changed, 20 insertions(+), 4 deletions(-)
>>
>
> AFAIK, all callers have to report an error message if port == 0. As we
> see from these missing error reports, this is fragile. Lets just make
> virPortAllocatorAcquire() return -1 if the ports are exhausted and
> report an error itself. This removes the chance that the caller will
> forget.
It will also remove the specific error message. It might be helpful to tell
the user which port range is exhausted.
Could you pass the "name" into the constructor of virPortAllocator
object, so it is available when reporting errors ?
Regards,
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 :|
4041
days inactive
4041
days old
5 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Ján Tomko