VirutalBox has a IVRDEServerInfo structure available that
gives the effective runtime port that the VM is using when it's
running. This is useful when the "TCP/Ports" VBox property was set to
port range (e.g. via autoport = "yes" or via VBoxManage) in which
case it would be impossible to get the "active" port otherwise.
---
src/vbox/vbox_common.c | 3 +-
src/vbox/vbox_tmpl.c | 134 +++++++++++++++++++++++++++++++-----------
src/vbox/vbox_uniformed_api.h | 2 +-
3 files changed, 104 insertions(+), 35 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 92ee37164..d542f2b76 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3326,7 +3326,8 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
if (VIR_ALLOC(graphics) < 0)
goto cleanup;
- gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, graphics);
+ gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, machine, graphics);
+ gVBoxAPI.UISession.Close(data->vboxSession);
graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 2b3f2e3eb..c7682f13c 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -221,29 +221,6 @@ _vboxIIDFromArrayItem(vboxDriverPtr data, vboxIID *iid,
_vboxIIDFromArrayItem(data, iid, array, idx)
#define DEBUGIID(msg, strUtf16) DEBUGPRUnichar(msg, strUtf16)
-/**
- * Converts Utf-16 string to int
- */
-static int PRUnicharToInt(PCVBOXXPCOM pFuncs, PRUnichar *strUtf16)
-{
- char *strUtf8 = NULL;
- int ret = 0;
-
- if (!strUtf16)
- return -1;
-
- pFuncs->pfnUtf16ToUtf8(strUtf16, &strUtf8);
- if (!strUtf8)
- return -1;
-
- if (virStrToLong_i(strUtf8, NULL, 10, &ret) < 0)
- ret = -1;
-
- pFuncs->pfnUtf8Free(strUtf8);
-
- return ret;
-}
-
/**
* Converts int to Utf-16 string
*/
@@ -280,6 +257,54 @@ static virDomainState _vboxConvertState(PRUint32 state)
}
}
+
+static int
+vboxGetActiveVRDEServerPort(ISession *session, IMachine *machine)
+{
+ nsresult rc;
+ PRInt32 port = -1;
+ IVRDEServerInfo *vrdeInfo = NULL;
+ IConsole *console = NULL;
+
+ rc = machine->vtbl->LockMachine(machine, session, LockType_Shared);
+ if (NS_FAILED(rc)) {
+ VIR_WARN("Could not obtain shared lock on VBox VM, rc=%08x", rc);
+ return -1;
+ }
+
+ rc = session->vtbl->GetConsole(session, &console);
+ if (NS_FAILED(rc)) {
+ VIR_WARN("Could not get VBox session console, rc=%08x", rc);
+ goto cleanup;
+ }
+
+ /* it may be null if VM is not running */
+ if (!console)
+ goto cleanup;
+
+ rc = console->vtbl->GetVRDEServerInfo(console, &vrdeInfo);
+
+ if (NS_FAILED(rc) || !vrdeInfo) {
+ VIR_WARN("Could not get VBox VM VRDEServerInfo, rc=%08x", rc);
+ goto cleanup;
+ }
+
+ rc = vrdeInfo->vtbl->GetPort(vrdeInfo, &port);
+
+ if (NS_FAILED(rc)) {
+ VIR_WARN("Could not read port from VRDEServerInfo, rc=%08x", rc);
+ goto cleanup;
+ }
+
+ cleanup:
+ VBOX_RELEASE(console);
+ VBOX_RELEASE(vrdeInfo);
+ session->vtbl->UnlockMachine(session);
+
+ return port;
+}
+
+
static int
_vboxDomainSnapshotRestore(virDomainPtr dom,
IMachine *machine,
@@ -1576,24 +1601,67 @@ _vrdeServerSetEnabled(IVRDEServer *VRDEServer, PRBool enabled)
}
static nsresult
-_vrdeServerGetPorts(vboxDriverPtr data ATTRIBUTE_UNUSED,
- IVRDEServer *VRDEServer, virDomainGraphicsDefPtr graphics)
+_vrdeServerGetPorts(vboxDriverPtr data, IVRDEServer *VRDEServer,
+ IMachine *machine, virDomainGraphicsDefPtr graphics)
{
nsresult rc;
PRUnichar *VRDEPortsKey = NULL;
PRUnichar *VRDEPortsValue = NULL;
+ PRInt32 port = -1;
+ ssize_t nmatches = 0;
+ char **matches = NULL;
+ char *portUtf8 = NULL;
+
+ /* get active (effective) port - available only when VM is running and has
+ * the VBOX extensions installed (without extenstions RDP server
+ * functionality is disabled)
+ */
+ port = vboxGetActiveVRDEServerPort(data->vboxSession, machine);
+ if (port > 0)
+ graphics->data.rdp.port = port;
+
+ /* get the port (or port range) set in VM properties, this info will
+ * be used to determine whether to set autoport flag
+ */
VBOX_UTF8_TO_UTF16("TCP/Ports", &VRDEPortsKey);
- rc = VRDEServer->vtbl->GetVRDEProperty(VRDEServer, VRDEPortsKey, &VRDEPortsValue);
- VBOX_UTF16_FREE(VRDEPortsKey);
- if (VRDEPortsValue) {
- /* even if vbox supports mutilpe ports, single port for now here */
- graphics->data.rdp.port = PRUnicharToInt(data->pFuncs, VRDEPortsValue);
- VBOX_UTF16_FREE(VRDEPortsValue);
- } else {
- graphics->data.rdp.autoport = true;
+ rc = VRDEServer->vtbl->GetVRDEProperty(VRDEServer, VRDEPortsKey,
+ &VRDEPortsValue);
+
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to read RDP port value, rc=%08x"),
+ (unsigned) rc);
+ goto cleanup;
}
+ VBOX_UTF16_TO_UTF8(VRDEPortsValue, &portUtf8);
+
+ if (portUtf8) {
+ /* does the string contain digits only */
+ nmatches = virStringSearch(portUtf8, "(^[[:digit:]]+$)", 1, &matches);
+
+ /* the port property is not numeric, then it must be a port range or
+ * port list or combination of the two, either way it's an autoport
+ */
+ if (nmatches != 1)
+ graphics->data.rdp.autoport = true;
+
+ /* no active port available, e.g. VM is powered off, try to get it from
+ * the property string
+ */
+ if (port < 0) {
+ if (nmatches == 1 && virStrToLong_i(portUtf8, NULL, 10, &port) == 0)
+ graphics->data.rdp.port = port;
+ }
+ }
+
+ cleanup:
+ virStringListFree(matches);
+ VBOX_UTF8_FREE(portUtf8);
+ VBOX_UTF16_FREE(VRDEPortsValue);
+ VBOX_UTF16_FREE(VRDEPortsKey);
+
return rc;
}
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
index 2ccaf43e8..8cf27789b 100644
--- a/src/vbox/vbox_uniformed_api.h
+++ b/src/vbox/vbox_uniformed_api.h
@@ -341,7 +341,7 @@ typedef struct {
nsresult (*GetEnabled)(IVRDEServer *VRDEServer, PRBool *enabled);
nsresult (*SetEnabled)(IVRDEServer *VRDEServer, PRBool enabled);
nsresult (*GetPorts)(vboxDriverPtr driver, IVRDEServer *VRDEServer,
- virDomainGraphicsDefPtr graphics);
+ IMachine *machine, virDomainGraphicsDefPtr graphics);
nsresult (*SetPorts)(vboxDriverPtr driver, IVRDEServer *VRDEServer,
virDomainGraphicsDefPtr graphics);
nsresult (*GetReuseSingleConnection)(IVRDEServer *VRDEServer, PRBool *enabled);
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 10/24/2017 05:09 PM, Dawid Zamirski wrote:
> VirutalBox has a IVRDEServerInfo structure available that
> gives the effective runtime port that the VM is using when it's
> running. This is useful when the "TCP/Ports" VBox property was set to
> port range (e.g. via autoport = "yes" or via VBoxManage) in which
> case it would be impossible to get the "active" port otherwise.
> ---
> src/vbox/vbox_common.c | 3 +-
> src/vbox/vbox_tmpl.c | 134 +++++++++++++++++++++++++++++++-----------
> src/vbox/vbox_uniformed_api.h | 2 +-
> 3 files changed, 104 insertions(+), 35 deletions(-)
>
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 92ee37164..d542f2b76 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -3326,7 +3326,8 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
> if (VIR_ALLOC(graphics) < 0)
> goto cleanup;
>
> - gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, graphics);
> + gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, machine, graphics);
> + gVBoxAPI.UISession.Close(data->vboxSession);
But @data is used shortly after this and I don't see in the calling tree
a corresponding UISession.Open* of some type or am I missing it in some
called function?
The rest looks good - just need to know about this. I can remove before
pushing or make some other sort of simple adjustment.
John
(I'm at KVM Forum in Prague - so normal work schedule is a bit off)
>
> graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 2b3f2e3eb..c7682f13c 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -221,29 +221,6 @@ _vboxIIDFromArrayItem(vboxDriverPtr data, vboxIID *iid,
> _vboxIIDFromArrayItem(data, iid, array, idx)
> #define DEBUGIID(msg, strUtf16) DEBUGPRUnichar(msg, strUtf16)
>
> -/**
> - * Converts Utf-16 string to int
> - */
> -static int PRUnicharToInt(PCVBOXXPCOM pFuncs, PRUnichar *strUtf16)
> -{
> - char *strUtf8 = NULL;
> - int ret = 0;
> -
> - if (!strUtf16)
> - return -1;
> -
> - pFuncs->pfnUtf16ToUtf8(strUtf16, &strUtf8);
> - if (!strUtf8)
> - return -1;
> -
> - if (virStrToLong_i(strUtf8, NULL, 10, &ret) < 0)
> - ret = -1;
> -
> - pFuncs->pfnUtf8Free(strUtf8);
> -
> - return ret;
> -}
> -
> /**
> * Converts int to Utf-16 string
> */
> @@ -280,6 +257,54 @@ static virDomainState _vboxConvertState(PRUint32 state)
> }
> }
>
> +
> +static int
> +vboxGetActiveVRDEServerPort(ISession *session, IMachine *machine)
> +{
> + nsresult rc;
> + PRInt32 port = -1;
> + IVRDEServerInfo *vrdeInfo = NULL;
> + IConsole *console = NULL;
> +
> + rc = machine->vtbl->LockMachine(machine, session, LockType_Shared);
> + if (NS_FAILED(rc)) {
> + VIR_WARN("Could not obtain shared lock on VBox VM, rc=%08x", rc);
> + return -1;
> + }
> +
> + rc = session->vtbl->GetConsole(session, &console);
> + if (NS_FAILED(rc)) {
> + VIR_WARN("Could not get VBox session console, rc=%08x", rc);
> + goto cleanup;
> + }
> +
> + /* it may be null if VM is not running */
> + if (!console)
> + goto cleanup;
> +
> + rc = console->vtbl->GetVRDEServerInfo(console, &vrdeInfo);
> +
> + if (NS_FAILED(rc) || !vrdeInfo) {
> + VIR_WARN("Could not get VBox VM VRDEServerInfo, rc=%08x", rc);
> + goto cleanup;
> + }
> +
> + rc = vrdeInfo->vtbl->GetPort(vrdeInfo, &port);
> +
> + if (NS_FAILED(rc)) {
> + VIR_WARN("Could not read port from VRDEServerInfo, rc=%08x", rc);
> + goto cleanup;
> + }
> +
> + cleanup:
> + VBOX_RELEASE(console);
> + VBOX_RELEASE(vrdeInfo);
> + session->vtbl->UnlockMachine(session);
> +
> + return port;
> +}
> +
> +
> static int
> _vboxDomainSnapshotRestore(virDomainPtr dom,
> IMachine *machine,
> @@ -1576,24 +1601,67 @@ _vrdeServerSetEnabled(IVRDEServer *VRDEServer, PRBool enabled)
> }
>
> static nsresult
> -_vrdeServerGetPorts(vboxDriverPtr data ATTRIBUTE_UNUSED,
> - IVRDEServer *VRDEServer, virDomainGraphicsDefPtr graphics)
> +_vrdeServerGetPorts(vboxDriverPtr data, IVRDEServer *VRDEServer,
> + IMachine *machine, virDomainGraphicsDefPtr graphics)
> {
> nsresult rc;
> PRUnichar *VRDEPortsKey = NULL;
> PRUnichar *VRDEPortsValue = NULL;
> + PRInt32 port = -1;
> + ssize_t nmatches = 0;
> + char **matches = NULL;
> + char *portUtf8 = NULL;
> +
> + /* get active (effective) port - available only when VM is running and has
> + * the VBOX extensions installed (without extenstions RDP server
> + * functionality is disabled)
> + */
> + port = vboxGetActiveVRDEServerPort(data->vboxSession, machine);
>
> + if (port > 0)
> + graphics->data.rdp.port = port;
> +
> + /* get the port (or port range) set in VM properties, this info will
> + * be used to determine whether to set autoport flag
> + */
> VBOX_UTF8_TO_UTF16("TCP/Ports", &VRDEPortsKey);
> - rc = VRDEServer->vtbl->GetVRDEProperty(VRDEServer, VRDEPortsKey, &VRDEPortsValue);
> - VBOX_UTF16_FREE(VRDEPortsKey);
> - if (VRDEPortsValue) {
> - /* even if vbox supports mutilpe ports, single port for now here */
> - graphics->data.rdp.port = PRUnicharToInt(data->pFuncs, VRDEPortsValue);
> - VBOX_UTF16_FREE(VRDEPortsValue);
> - } else {
> - graphics->data.rdp.autoport = true;
> + rc = VRDEServer->vtbl->GetVRDEProperty(VRDEServer, VRDEPortsKey,
> + &VRDEPortsValue);
> +
> + if (NS_FAILED(rc)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to read RDP port value, rc=%08x"),
> + (unsigned) rc);
> + goto cleanup;
> }
>
> + VBOX_UTF16_TO_UTF8(VRDEPortsValue, &portUtf8);
> +
> + if (portUtf8) {
> + /* does the string contain digits only */
> + nmatches = virStringSearch(portUtf8, "(^[[:digit:]]+$)", 1, &matches);
> +
> + /* the port property is not numeric, then it must be a port range or
> + * port list or combination of the two, either way it's an autoport
> + */
> + if (nmatches != 1)
> + graphics->data.rdp.autoport = true;
> +
> + /* no active port available, e.g. VM is powered off, try to get it from
> + * the property string
> + */
> + if (port < 0) {
> + if (nmatches == 1 && virStrToLong_i(portUtf8, NULL, 10, &port) == 0)
> + graphics->data.rdp.port = port;
> + }
> + }
> +
> + cleanup:
> + virStringListFree(matches);
> + VBOX_UTF8_FREE(portUtf8);
> + VBOX_UTF16_FREE(VRDEPortsValue);
> + VBOX_UTF16_FREE(VRDEPortsKey);
> +
> return rc;
> }
>
> diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
> index 2ccaf43e8..8cf27789b 100644
> --- a/src/vbox/vbox_uniformed_api.h
> +++ b/src/vbox/vbox_uniformed_api.h
> @@ -341,7 +341,7 @@ typedef struct {
> nsresult (*GetEnabled)(IVRDEServer *VRDEServer, PRBool *enabled);
> nsresult (*SetEnabled)(IVRDEServer *VRDEServer, PRBool enabled);
> nsresult (*GetPorts)(vboxDriverPtr driver, IVRDEServer *VRDEServer,
> - virDomainGraphicsDefPtr graphics);
> + IMachine *machine, virDomainGraphicsDefPtr graphics);
> nsresult (*SetPorts)(vboxDriverPtr driver, IVRDEServer *VRDEServer,
> virDomainGraphicsDefPtr graphics);
> nsresult (*GetReuseSingleConnection)(IVRDEServer *VRDEServer, PRBool *enabled);
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2017-10-25 at 17:35 -0400, John Ferlan wrote: > > On 10/24/2017 05:09 PM, Dawid Zamirski wrote: > > VirutalBox has a IVRDEServerInfo structure available that > > gives the effective runtime port that the VM is using when it's > > running. This is useful when the "TCP/Ports" VBox property was set > > to > > port range (e.g. via autoport = "yes" or via VBoxManage) in which > > case it would be impossible to get the "active" port otherwise. > > --- > > src/vbox/vbox_common.c | 3 +- > > src/vbox/vbox_tmpl.c | 134 > > +++++++++++++++++++++++++++++++----------- > > src/vbox/vbox_uniformed_api.h | 2 +- > > 3 files changed, 104 insertions(+), 35 deletions(-) > > > > diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c > > index 92ee37164..d542f2b76 100644 > > --- a/src/vbox/vbox_common.c > > +++ b/src/vbox/vbox_common.c > > @@ -3326,7 +3326,8 @@ vboxDumpDisplay(virDomainDefPtr def, > > vboxDriverPtr data, IMachine *machine) > > if (VIR_ALLOC(graphics) < 0) > > goto cleanup; > > > > - gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, > > graphics); > > + gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, machine, > > graphics); > > + gVBoxAPI.UISession.Close(data->vboxSession); > > But @data is used shortly after this and I don't see in the calling > tree > a corresponding UISession.Open* of some type or am I missing it in > some > called function? > > > The rest looks good - just need to know about this. I can remove > before > pushing or make some other sort of simple adjustment. Yep this should be removed - it's a left over from my old internal patch [1], that I forgot to remove when preparing for upstream submission. It was originally preceded with OpenExisting (aka LockMachine) to get the IConsole - the new patch does it internally in the vboxGetActiveVRDEServerPort function. https://github.com/datto/libvirt/commit/a3cb830bfce10b1a614c18a6ac50783 45433d900#diff-747d3af65e7ac81a564b7cb4fcd01eb6R3516 Thank you, Dawid > > John > > (I'm at KVM Forum in Prague - so normal work schedule is a bit off) > > > > > > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 10/25/2017 05:54 PM, Dawid Zamirski wrote: > On Wed, 2017-10-25 at 17:35 -0400, John Ferlan wrote: >> >> On 10/24/2017 05:09 PM, Dawid Zamirski wrote: >>> VirutalBox has a IVRDEServerInfo structure available that >>> gives the effective runtime port that the VM is using when it's >>> running. This is useful when the "TCP/Ports" VBox property was set >>> to >>> port range (e.g. via autoport = "yes" or via VBoxManage) in which >>> case it would be impossible to get the "active" port otherwise. >>> --- >>> src/vbox/vbox_common.c | 3 +- >>> src/vbox/vbox_tmpl.c | 134 >>> +++++++++++++++++++++++++++++++----------- >>> src/vbox/vbox_uniformed_api.h | 2 +- >>> 3 files changed, 104 insertions(+), 35 deletions(-) >>> >>> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c >>> index 92ee37164..d542f2b76 100644 >>> --- a/src/vbox/vbox_common.c >>> +++ b/src/vbox/vbox_common.c >>> @@ -3326,7 +3326,8 @@ vboxDumpDisplay(virDomainDefPtr def, >>> vboxDriverPtr data, IMachine *machine) >>> if (VIR_ALLOC(graphics) < 0) >>> goto cleanup; >>> >>> - gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, >>> graphics); >>> + gVBoxAPI.UIVRDEServer.GetPorts(data, VRDEServer, machine, >>> graphics); >>> + gVBoxAPI.UISession.Close(data->vboxSession); >> >> But @data is used shortly after this and I don't see in the calling >> tree >> a corresponding UISession.Open* of some type or am I missing it in >> some >> called function? >> >> >> The rest looks good - just need to know about this. I can remove >> before >> pushing or make some other sort of simple adjustment. > > Yep this should be removed - it's a left over from my old internal > patch [1], that I forgot to remove when preparing for upstream > submission. It was originally preceded with OpenExisting (aka > LockMachine) to get the IConsole - the new patch does it internally in > the vboxGetActiveVRDEServerPort function. > > https://github.com/datto/libvirt/commit/a3cb830bfce10b1a614c18a6ac50783 > 45433d900#diff-747d3af65e7ac81a564b7cb4fcd01eb6R3516 > > Thank you, > Dawid > Reviewed-by: John Ferlan <jferlan@redhat.com> John (pushed now too) >> >> John >> >> (I'm at KVM Forum in Prague - so normal work schedule is a bit off) >> >>> >>> >>> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.