[libvirt] [PATCH 0/5] graphics cleanup and bug fix
Pavel Hrdina (5): qemu_process: graphics: ref driver config only in function where it is used qemu_process: graphics: extract port allocation into function qemu_process: graphics: extract for loop out of qemuProcessGraphicsReservePorts qemu_process: graphics: reserve port only if listen type is address or network qemu_process: graphics: setup listen types before ports are reserved/allocated src/qemu/qemu_process.c | 174 ++++++++++++++++++++++++++++++------------------ 1 file changed, 108 insertions(+), 66 deletions(-) -- 2.9.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com --- src/qemu/qemu_process.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7481626..26aef5e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3545,14 +3545,15 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, static int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, - virQEMUDriverConfigPtr cfg, virDomainGraphicsDefPtr graphics, bool allocate) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned short port = 0; unsigned short tlsPort; size_t i; int defaultMode = graphics->data.spice.defaultMode; + int ret = -1; bool needTLSPort = false; bool needPort = false; @@ -3598,12 +3599,13 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, if (needTLSPort || graphics->data.spice.tlsPort == -1) graphics->data.spice.tlsPort = 5902; - return 0; + ret = 0; + goto cleanup; } if (needPort || graphics->data.spice.port == -1) { if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) - goto error; + goto cleanup; graphics->data.spice.port = port; @@ -3616,11 +3618,11 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Auto allocation of spice TLS port requested " "but spice TLS is disabled in qemu.conf")); - goto error; + goto cleanup; } if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) - goto error; + goto cleanup; graphics->data.spice.tlsPort = tlsPort; @@ -3628,11 +3630,12 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, graphics->data.spice.tlsPortReserved = true; } - return 0; + ret = 0; - error: + cleanup: virPortAllocatorRelease(driver->remotePorts, port); - return -1; + virObjectUnref(cfg); + return ret; } @@ -4070,15 +4073,17 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, static int -qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, +qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *type = virDomainGraphicsTypeToString(graphics->type); char *listenAddr = NULL; bool useSocket = false; size_t i; + int ret = -1; switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: @@ -4111,12 +4116,12 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, memset(glisten, 0, sizeof(virDomainGraphicsListenDef)); if (virAsprintf(&glisten->socket, "%s/%s.sock", priv->libDir, type) < 0) - return -1; + goto cleanup; glisten->fromConfig = true; glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET; } else if (listenAddr) { if (VIR_STRDUP(glisten->address, listenAddr) < 0) - return -1; + goto cleanup; glisten->fromConfig = true; } } @@ -4128,14 +4133,14 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, if (qemuProcessGraphicsSetupNetworkAddress(glisten, listenAddr) < 0) - return -1; + goto cleanup; break; case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: if (!glisten->socket) { if (virAsprintf(&glisten->socket, "%s/%s.sock", priv->libDir, type) < 0) - return -1; + goto cleanup; glisten->autoGenerated = true; } break; @@ -4146,7 +4151,11 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, } } - return 0; + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; } @@ -4155,7 +4164,6 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool allocate = !(flags & VIR_QEMU_PROCESS_START_PRETEND); size_t i; int ret = -1; @@ -4176,8 +4184,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, - allocate) < 0) + if (qemuProcessSPICEAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; break; @@ -4189,14 +4196,13 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, } } - if (qemuProcessGraphicsSetupListen(cfg, graphics, vm) < 0) + if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) goto cleanup; } ret = 0; cleanup: - virObjectUnref(cfg); return ret; } -- 2.9.2
On 08/15/2016 07:28 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com --- src/qemu/qemu_process.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-)
ACK - couple of nits below to consider... John
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7481626..26aef5e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3545,14 +3545,15 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
static int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, - virQEMUDriverConfigPtr cfg, virDomainGraphicsDefPtr graphics, bool allocate) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned short port = 0; unsigned short tlsPort;
While not necessary yet, future adjustment proofing makes me wonder if it's worth it to initialize this to 0 and the corresponding virPortAllocatorRelease call added in cleanup?
size_t i; int defaultMode = graphics->data.spice.defaultMode; + int ret = -1;
bool needTLSPort = false; bool needPort = false; @@ -3598,12 +3599,13 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, if (needTLSPort || graphics->data.spice.tlsPort == -1) graphics->data.spice.tlsPort = 5902;
- return 0; + ret = 0; + goto cleanup; }
if (needPort || graphics->data.spice.port == -1) { if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) - goto error; + goto cleanup;
graphics->data.spice.port = port;
@@ -3616,11 +3618,11 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Auto allocation of spice TLS port requested " "but spice TLS is disabled in qemu.conf")); - goto error; + goto cleanup; }
if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) - goto error; + goto cleanup;
graphics->data.spice.tlsPort = tlsPort;
@@ -3628,11 +3630,12 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, graphics->data.spice.tlsPortReserved = true; }
- return 0; + ret = 0;
- error: + cleanup: virPortAllocatorRelease(driver->remotePorts, port); - return -1; + virObjectUnref(cfg); + return ret; }
@@ -4070,15 +4073,17 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten,
static int -qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, +qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *type = virDomainGraphicsTypeToString(graphics->type); char *listenAddr = NULL; bool useSocket = false; size_t i; + int ret = -1;
switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
"Technically" after this switch cfg isn't used, so it could be unref'd avoiding the cleanup changes...
@@ -4111,12 +4116,12 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, memset(glisten, 0, sizeof(virDomainGraphicsListenDef)); if (virAsprintf(&glisten->socket, "%s/%s.sock", priv->libDir, type) < 0) - return -1; + goto cleanup; glisten->fromConfig = true; glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET; } else if (listenAddr) { if (VIR_STRDUP(glisten->address, listenAddr) < 0) - return -1; + goto cleanup; glisten->fromConfig = true; } } @@ -4128,14 +4133,14 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg,
if (qemuProcessGraphicsSetupNetworkAddress(glisten, listenAddr) < 0) - return -1; + goto cleanup; break;
case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: if (!glisten->socket) { if (virAsprintf(&glisten->socket, "%s/%s.sock", priv->libDir, type) < 0) - return -1; + goto cleanup; glisten->autoGenerated = true; } break; @@ -4146,7 +4151,11 @@ qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, } }
- return 0; + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; }
@@ -4155,7 +4164,6 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool allocate = !(flags & VIR_QEMU_PROCESS_START_PRETEND); size_t i; int ret = -1; @@ -4176,8 +4184,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, break;
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, - allocate) < 0) + if (qemuProcessSPICEAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; break;
@@ -4189,14 +4196,13 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, } }
- if (qemuProcessGraphicsSetupListen(cfg, graphics, vm) < 0) + if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) goto cleanup; }
ret = 0;
cleanup: - virObjectUnref(cfg); return ret; }
On Tue, Aug 16, 2016 at 06:32:51PM -0400, John Ferlan wrote:
On 08/15/2016 07:28 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com --- src/qemu/qemu_process.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-)
ACK - couple of nits below to consider...
John
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7481626..26aef5e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3545,14 +3545,15 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
static int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, - virQEMUDriverConfigPtr cfg, virDomainGraphicsDefPtr graphics, bool allocate) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned short port = 0; unsigned short tlsPort;
While not necessary yet, future adjustment proofing makes me wonder if it's worth it to initialize this to 0 and the corresponding virPortAllocatorRelease call added in cleanup?
Sure, this can be done as a follow up.
size_t i; int defaultMode = graphics->data.spice.defaultMode; + int ret = -1;
bool needTLSPort = false; bool needPort = false; @@ -3598,12 +3599,13 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, if (needTLSPort || graphics->data.spice.tlsPort == -1) graphics->data.spice.tlsPort = 5902;
- return 0; + ret = 0; + goto cleanup; }
if (needPort || graphics->data.spice.port == -1) { if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) - goto error; + goto cleanup;
graphics->data.spice.port = port;
@@ -3616,11 +3618,11 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Auto allocation of spice TLS port requested " "but spice TLS is disabled in qemu.conf")); - goto error; + goto cleanup; }
if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) - goto error; + goto cleanup;
graphics->data.spice.tlsPort = tlsPort;
@@ -3628,11 +3630,12 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, graphics->data.spice.tlsPortReserved = true; }
- return 0; + ret = 0;
- error: + cleanup: virPortAllocatorRelease(driver->remotePorts, port); - return -1; + virObjectUnref(cfg); + return ret; }
@@ -4070,15 +4073,17 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten,
static int -qemuProcessGraphicsSetupListen(virQEMUDriverConfigPtr cfg, +qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *type = virDomainGraphicsTypeToString(graphics->type); char *listenAddr = NULL; bool useSocket = false; size_t i; + int ret = -1;
switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
"Technically" after this switch cfg isn't used, so it could be unref'd avoiding the cleanup changes...
Well, yes that's true but as future proofing I would rather use cleanup in case that someone uses *cfg* later on in this function. Pavel
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 61 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 26aef5e..f813bae 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4046,6 +4046,44 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, static int +qemuProcessGraphicsAllocatePorts(virQEMUDriverPtr driver, + virDomainGraphicsDefPtr graphics, + bool allocate) +{ + virDomainGraphicsListenDefPtr glisten; + + if (graphics->nListens <= 0) + return 0; + + glisten = &graphics->listens[0]; + + if (glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && + glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) + return 0; + + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (qemuProcessVNCAllocatePorts(driver, graphics, allocate) < 0) + return -1; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (qemuProcessSPICEAllocatePorts(driver, graphics, allocate) < 0) + return -1; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; + } + + return 0; +} + + +static int qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, const char *listenAddr) { @@ -4174,27 +4212,8 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->nListens > 0 && - (graphics->listens[0].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS || - graphics->listens[0].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { - switch (graphics->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if (qemuProcessVNCAllocatePorts(driver, graphics, allocate) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (qemuProcessSPICEAllocatePorts(driver, graphics, allocate) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - break; - } - } + if (qemuProcessGraphicsAllocatePorts(driver, graphics, allocate) < 0) + goto cleanup; if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) goto cleanup; -- 2.9.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 58 +++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f813bae..d61f704 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4007,37 +4007,32 @@ qemuProcessStartHook(virQEMUDriverPtr driver, static int qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, - virDomainObjPtr vm) + virDomainGraphicsDefPtr graphics) { - size_t i; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.autoport) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.vnc.port, + true) < 0) + return -1; + graphics->data.vnc.portReserved = true; - for (i = 0; i < vm->def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !graphics->data.vnc.autoport) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + !graphics->data.spice.autoport) { + if (graphics->data.spice.port > 0) { if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.vnc.port, + graphics->data.spice.port, true) < 0) return -1; - graphics->data.vnc.portReserved = true; - - } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - !graphics->data.spice.autoport) { - if (graphics->data.spice.port > 0) { - if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.spice.port, - true) < 0) - return -1; - graphics->data.spice.portReserved = true; - } + graphics->data.spice.portReserved = true; + } - if (graphics->data.spice.tlsPort > 0) { - if (virPortAllocatorSetUsed(driver->remotePorts, - graphics->data.spice.tlsPort, - true) < 0) - return -1; - graphics->data.spice.tlsPortReserved = true; - } + if (graphics->data.spice.tlsPort > 0) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.tlsPort, + true) < 0) + return -1; + graphics->data.spice.tlsPortReserved = true; } } @@ -4202,15 +4197,22 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { + virDomainGraphicsDefPtr graphics; bool allocate = !(flags & VIR_QEMU_PROCESS_START_PRETEND); size_t i; int ret = -1; - if (allocate && qemuProcessGraphicsReservePorts(driver, vm) < 0) - goto cleanup; + if (allocate) { + for (i = 0; i < vm->def->ngraphics; i++) { + graphics = vm->def->graphics[i]; + + if (qemuProcessGraphicsReservePorts(driver, graphics) < 0) + goto cleanup; + } + } for (i = 0; i < vm->def->ngraphics; ++i) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + graphics = vm->def->graphics[i]; if (qemuProcessGraphicsAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; -- 2.9.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d61f704..b3c6400 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4009,6 +4009,17 @@ static int qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics) { + virDomainGraphicsListenDefPtr glisten; + + if (graphics->nListens <= 0) + return 0; + + glisten = &graphics->listens[0]; + + if (glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && + glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) + return 0; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !graphics->data.vnc.autoport) { if (virPortAllocatorSetUsed(driver->remotePorts, -- 2.9.2
Could use a little bit of explanation here! Details as to why we can ignore those glisten types... John On 08/15/2016 07:28 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d61f704..b3c6400 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4009,6 +4009,17 @@ static int qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics) { + virDomainGraphicsListenDefPtr glisten; + + if (graphics->nListens <= 0) + return 0; + + glisten = &graphics->listens[0]; + + if (glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS && + glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) + return 0; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !graphics->data.vnc.autoport) { if (virPortAllocatorSetUsed(driver->remotePorts,
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1364843 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b3c6400..2b857bb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4213,6 +4213,13 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, size_t i; int ret = -1; + for (i = 0; i < vm->def->ngraphics; i++) { + graphics = vm->def->graphics[i]; + + if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) + goto cleanup; + } + if (allocate) { for (i = 0; i < vm->def->ngraphics; i++) { graphics = vm->def->graphics[i]; @@ -4227,9 +4234,6 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, if (qemuProcessGraphicsAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; - - if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) - goto cleanup; } ret = 0; -- 2.9.2
On 08/15/2016 07:28 AM, Pavel Hrdina wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1364843
Similar to 4/5 a few extra words here would help so one doesn't have to chase into the bz to attempt to figure out what's being fixed and why moving the Setup earlier does what you want. ACK series - please do add a few more commit words though John
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b3c6400..2b857bb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4213,6 +4213,13 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, size_t i; int ret = -1;
+ for (i = 0; i < vm->def->ngraphics; i++) { + graphics = vm->def->graphics[i]; + + if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) + goto cleanup; + } + if (allocate) { for (i = 0; i < vm->def->ngraphics; i++) { graphics = vm->def->graphics[i]; @@ -4227,9 +4234,6 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver,
if (qemuProcessGraphicsAllocatePorts(driver, graphics, allocate) < 0) goto cleanup; - - if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) - goto cleanup; }
ret = 0;
On Tue, Aug 16, 2016 at 06:35:17PM -0400, John Ferlan wrote:
On 08/15/2016 07:28 AM, Pavel Hrdina wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1364843
Similar to 4/5 a few extra words here would help so one doesn't have to chase into the bz to attempt to figure out what's being fixed and why moving the Setup earlier does what you want.
ACK series - please do add a few more commit words though
Thanks, I'll add some words to commit messages and push it shortly. Pavel
participants (2)
-
John Ferlan -
Pavel Hrdina