Re: [libvirt] [PATCH 2/3] vbox: Read runtime RDP port and handle autoport.
On 10/10/2017 05:52 PM, Dawid Zamirski wrote:
> VirutalBox has a IVRDEServerInfo sturcture 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 | 135
> +-
> src/vbox/vbox_uniformed_api.h | 2 +-
> 3 files changed, 97 insertions(+), 43 deletions(-)
>
Is there more than one change going on here?
The removal of VBOX_SESSION_{OPEN|CLOSE} could seemingly be a separately
explained patch...
> 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 8e47d90d6..ff69cf39c 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -144,12 +144,6 @@ if (strUtf16) {\
>(unsigned)(iid)->m3[7]);\
> }\
>
> -#define VBOX_SESSION_OPEN(/* unused */ iid_value, /* in */ machine) \
> -machine->vtbl->LockMachine(machine, data->vboxSession, LockType_Write)
> -
> -#define VBOX_SESSION_CLOSE() \
> -data->vboxSession->vtbl->UnlockMachine(data->vboxSession)
> -
> #define VBOX_IID_INITIALIZER { NULL, true }
>
> /* default RDP port range to use for auto-port setting */
> @@ -227,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, );
> -if (!strUtf8)
> -return -1;
> -
> -if (virStrToLong_i(strUtf8, NULL, 10, ) < 0)
> -ret = -1;
> -
> -pFuncs->pfnUtf8Free(strUtf8);
> -
> -return ret;
> -}
> -
> /**
> * Converts int to Utf-16 string
> */
> @@ -286,6 +257,56 @@ static virDomainState _vboxConvertState(PRUint32 state)
> }
> }
>
> +static int
> +vboxGetActiveVRDEServerPort(ISession *session, IMachine *machine)
> +{
> +nsresult rc;
> +PRInt32 port = -1;
> +IVRDEServerInfo *vrdeInfo = NULL;
> +IConsole *console = NULL;
> +int locked = 0;
> +
> +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);
> +goto cleanup;
Why not just return -1 here since cleanup wouldn't need to clean up
@console or @vrdeInfo
That way @locked isn't necessary either.
> +} else {
> +locked = 1;
> +}
> +
> +rc = session->vtbl->GetConsole(session, );
> +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, );
> +
> +if (NS_FAILED(rc) || !vrdeInfo) {
> +VIR_WARN("Could not get VBox VM VRDEServerInfo, rc=%08x", rc);
> +goto cleanup;
> +}
> +
> +rc = vrdeInfo->vtbl->GetPort(vrdeInfo, );
> +
> +if (NS_FAILED(rc)) {
> +VIR_WARN("Could not read port from VRDEServerInfo, rc=%08x", rc);
> +goto cleanup;
> +}
> +
> + cleanup:
> +VBOX_RELEASE(console);
> +VBOX_RELEASE(vrdeInfo);
> +if (locked)
> +session->vtbl->UnlockMachine(session);
> +
> +return port;
> +}
> +
> static int
> _vboxDomainSnapshotRestore(virDomainPtr dom,
>IMachine *machine,
> @@ -326,7 +347,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom,
> goto cleanup;
> }
>
> -rc = VBOX_SESSION_OPEN(domiid.value, machine);
> +rc = machine->vtbl->LockMachine(machine, data->vboxSession,
> LockType_Write);
> #if VBOX_API_VERSION < 500
> if (NS_SUCCEEDED(rc))
> rc = data->vboxSession->vtbl->GetConsole(data->vboxSession,
> );
> @@ -371,7 +392,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom,
> #if VBOX_API_VERSION < 500
> VBOX_RELEASE(console);
> #endif /*VBOX_API_VERSION < 500*/
> -VBOX_SESSION_CLOSE();
> +
[libvirt] [PATCH 2/3] vbox: Read runtime RDP port and handle autoport.
VirutalBox has a IVRDEServerInfo sturcture 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 | 135 +-
src/vbox/vbox_uniformed_api.h | 2 +-
3 files changed, 97 insertions(+), 43 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 8e47d90d6..ff69cf39c 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -144,12 +144,6 @@ if (strUtf16) {\
(unsigned)(iid)->m3[7]);\
}\
-#define VBOX_SESSION_OPEN(/* unused */ iid_value, /* in */ machine) \
-machine->vtbl->LockMachine(machine, data->vboxSession, LockType_Write)
-
-#define VBOX_SESSION_CLOSE() \
-data->vboxSession->vtbl->UnlockMachine(data->vboxSession)
-
#define VBOX_IID_INITIALIZER { NULL, true }
/* default RDP port range to use for auto-port setting */
@@ -227,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, );
-if (!strUtf8)
-return -1;
-
-if (virStrToLong_i(strUtf8, NULL, 10, ) < 0)
-ret = -1;
-
-pFuncs->pfnUtf8Free(strUtf8);
-
-return ret;
-}
-
/**
* Converts int to Utf-16 string
*/
@@ -286,6 +257,56 @@ static virDomainState _vboxConvertState(PRUint32 state)
}
}
+static int
+vboxGetActiveVRDEServerPort(ISession *session, IMachine *machine)
+{
+nsresult rc;
+PRInt32 port = -1;
+IVRDEServerInfo *vrdeInfo = NULL;
+IConsole *console = NULL;
+int locked = 0;
+
+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);
+goto cleanup;
+} else {
+locked = 1;
+}
+
+rc = session->vtbl->GetConsole(session, );
+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, );
+
+if (NS_FAILED(rc) || !vrdeInfo) {
+VIR_WARN("Could not get VBox VM VRDEServerInfo, rc=%08x", rc);
+goto cleanup;
+}
+
+rc = vrdeInfo->vtbl->GetPort(vrdeInfo, );
+
+if (NS_FAILED(rc)) {
+VIR_WARN("Could not read port from VRDEServerInfo, rc=%08x", rc);
+goto cleanup;
+}
+
+ cleanup:
+VBOX_RELEASE(console);
+VBOX_RELEASE(vrdeInfo);
+if (locked)
+session->vtbl->UnlockMachine(session);
+
+return port;
+}
+
static int
_vboxDomainSnapshotRestore(virDomainPtr dom,
IMachine *machine,
@@ -326,7 +347,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom,
goto cleanup;
}
-rc = VBOX_SESSION_OPEN(domiid.value, machine);
+rc = machine->vtbl->LockMachine(machine, data->vboxSession,
LockType_Write);
#if VBOX_API_VERSION < 500
if (NS_SUCCEEDED(rc))
rc = data->vboxSession->vtbl->GetConsole(data->vboxSession, );
@@ -371,7 +392,7 @@ _vboxDomainSnapshotRestore(virDomainPtr dom,
#if VBOX_API_VERSION < 500
VBOX_RELEASE(console);
#endif /*VBOX_API_VERSION < 500*/
-VBOX_SESSION_CLOSE();
+data->vboxSession->vtbl->UnlockMachine(data->vboxSession);
vboxIIDUnalloc();
return ret;
}
@@ -1582,24 +1603,56 @@ _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
