[libvirt] [PATCH 1/1] Support for rdp/desktop types in graphics tag

[libvirt] [PATCH 3/3] networking...

[libvirt] [PATCH 0/2] Implement...

Pritesh Kothari

Thursday, 30 April 2009 Thu, 30 Apr '09

10:15 p.m.

Hi All, As per the discussion earlier on the list I have modified the rdp type and added a new desktop type and posting the patch for same. Regards, Pritesh

Attachments:

rdp-desktop_2009_04_30_1.patch (text/x-diff — 7.2 KB)

Show replies by thread

Daniel P. Berrange

Tuesday, 5 May Tue, 5 May

5:46 p.m.

On Thu, Apr 30, 2009 at 04:15:27PM +0200, Pritesh Kothari wrote:

...

Hi All, As per the discussion earlier on the list I have modified the rdp type and added a new desktop type and posting the patch for same.

ACK, this looks good to me. Daniel

...

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 2f784e1..5e4aa5b 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -711,6 +711,63 @@ </attribute> </optional> </group> + <group> + <attribute name="type"> + <value>rdp</value> + </attribute> + <optional> + <attribute name="port"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="autoport"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="replaceUser"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="multiUser"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="listen"> + <ref name="addrIP"/> + </attribute> + </optional> + </group> + <group> + <attribute name="type"> + <value>desktop</value> + </attribute> + <optional> + <attribute name="display"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="fullscreen"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + </group> </choice> </element> </define> diff --git a/src/domain_conf.c b/src/domain_conf.c index ed4b8e5..4030fa0 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -150,7 +150,9 @@ VIR_ENUM_IMPL(virDomainInputBus, VIR_DOMAIN_INPUT_BUS_LAST, VIR_ENUM_IMPL(virDomainGraphics, VIR_DOMAIN_GRAPHICS_TYPE_LAST, "sdl", - "vnc") + "vnc", + "rdp", + "desktop") VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, "subsystem", @@ -244,6 +246,14 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) VIR_FREE(def->data.sdl.display); VIR_FREE(def->data.sdl.xauth); break; + + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + VIR_FREE(def->data.rdp.listenAddr); + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + VIR_FREE(def->data.desktop.display); + break; } VIR_FREE(def); @@ -1501,6 +1511,68 @@ virDomainGraphicsDefParseXML(virConnectPtr conn, def->data.sdl.fullscreen = 0; def->data.sdl.xauth = virXMLPropString(node, "xauth"); def->data.sdl.display = virXMLPropString(node, "display"); + } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) { + char *port = virXMLPropString(node, "port"); + char *autoport; + char *replaceUser; + char *multiUser; + + if (port) { + if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot parse rdp port %s"), port); + VIR_FREE(port); + goto error; + } + VIR_FREE(port); + } else { + def->data.rdp.port = 0; + def->data.rdp.autoport = 1; + } + + if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + if (STREQ(autoport, "yes")) { + if (flags & VIR_DOMAIN_XML_INACTIVE) + def->data.rdp.port = 0; + def->data.rdp.autoport = 1; + } + VIR_FREE(autoport); + } + + if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) { + if (STREQ(replaceUser, "yes")) { + def->data.rdp.replaceUser = 1; + } + VIR_FREE(replaceUser); + } + + if ((multiUser = virXMLPropString(node, "multiUser")) != NULL) { + if (STREQ(multiUser, "yes")) { + def->data.rdp.multiUser = 1; + } + VIR_FREE(multiUser); + } + + def->data.rdp.listenAddr = virXMLPropString(node, "listen"); + } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP) { + char *fullscreen = virXMLPropString(node, "fullscreen"); + + if (fullscreen != NULL) { + if (STREQ(fullscreen, "yes")) { + def->data.desktop.fullscreen = 1; + } else if (STREQ(fullscreen, "no")) { + def->data.desktop.fullscreen = 0; + } else { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown fullscreen value '%s'"), fullscreen); + VIR_FREE(fullscreen); + goto error; + } + VIR_FREE(fullscreen); + } else + def->data.desktop.fullscreen = 0; + + def->data.desktop.display = virXMLPropString(node, "display"); } cleanup: @@ -3269,6 +3341,38 @@ virDomainGraphicsDefFormat(virConnectPtr conn, virBufferAddLit(buf, " fullscreen='yes'"); break; + + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + if (def->data.rdp.port) + virBufferVSprintf(buf, " port='%d'", + def->data.rdp.port); + else if (def->data.rdp.autoport) + virBufferAddLit(buf, " port='0'"); + + if (def->data.rdp.autoport) + virBufferVSprintf(buf, " autoport='yes'"); + + if (def->data.rdp.replaceUser) + virBufferVSprintf(buf, " replaceUser='yes'"); + + if (def->data.rdp.multiUser) + virBufferVSprintf(buf, " multiUser='yes'"); + + if (def->data.rdp.listenAddr) + virBufferVSprintf(buf, " listen='%s'", def->data.rdp.listenAddr); + + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + if (def->data.desktop.display) + virBufferEscapeString(buf, " display='%s'", + def->data.desktop.display); + + if (def->data.desktop.fullscreen) + virBufferAddLit(buf, " fullscreen='yes'"); + + break; + } virBufferAddLit(buf, "/>\n"); diff --git a/src/domain_conf.h b/src/domain_conf.h index 84fc477..1d12fea 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -264,6 +264,8 @@ struct _virDomainSoundDef { enum virDomainGraphicsType { VIR_DOMAIN_GRAPHICS_TYPE_SDL, VIR_DOMAIN_GRAPHICS_TYPE_VNC, + VIR_DOMAIN_GRAPHICS_TYPE_RDP, + VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP, VIR_DOMAIN_GRAPHICS_TYPE_LAST, }; @@ -285,6 +287,17 @@ struct _virDomainGraphicsDef { char *xauth; int fullscreen; } sdl; + struct { + int port; + char *listenAddr; + int autoport : 1; + int replaceUser : 1; + int multiUser : 1; + } rdp; + struct { + char *display; + int fullscreen : 1; + } desktop; } data; };

...

-- Libvir-list mailing list Libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Pritesh Kothari

Thursday, 7 May Thu, 7 May

9:49 p.m.

New subject: [libvirt] [PATCH 1/3] Support for vrdp/sdl/gui while defining a machine

Hi All, I have added support for vrdp/sdl/gui modes for VirtualBox driver in libvirt. Tha patch's are as below: [PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine. [PATCH 2/3]: contains support for vrdp/sdl/gui while dumping xml [PATCH 3/3]: contains support for vrdp/sdl/gui while starting the machine Regards, Pritesh

Pritesh Kothari

9:49 p.m.

New subject: [libvirt] [PATCH 2/3] Support for vrdp/sdl/gui while dumping xml

Pritesh Kothari

9:50 p.m.

New subject: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine

Daniel P. Berrange

Monday, 11 May Mon, 11 May

6:20 p.m.

New subject: [libvirt] [PATCH 1/3] Support for vrdp/sdl/gui while defining a machine

On Thu, May 07, 2009 at 03:49:28PM +0200, Pritesh Kothari wrote:

...

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index b25e93b..87db6ab 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3103,9 +3093,86 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) { VRDPServer->vtbl->nsisupports.Release((nsISupports *)VRDPServer); } } + + if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP) && (guiPresent == 0)) { + guiPresent = 1; + guiDisplay = strdup(def->graphics[i]->data.desktop.display); + } + + if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) && (sdlPresent == 0)) { + sdlPresent = 1; + sdlDisplay = strdup(def->graphics[i]->data.sdl.display); + } + }

Need to check for OOM failure here.

...

+ + if ((vrdpPresent == 1) && (guiPresent == 0) && (sdlPresent == 0)) { + /* store extradata key that frontend is set to vrdp */ + PRUnichar *keyTypeUtf16 = NULL; + PRUnichar *valueTypeUtf16 = NULL; + + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Type", &keyTypeUtf16); + data->pFuncs->pfnUtf8ToUtf16("vrdp", &valueTypeUtf16); + + machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16); + + data->pFuncs->pfnUtf16Free(keyTypeUtf16); + data->pFuncs->pfnUtf16Free(valueTypeUtf16); + + } else if ((guiPresent == 0) && (sdlPresent == 1)) { + /* store extradata key that frontend is set to sdl */ + PRUnichar *keyTypeUtf16 = NULL; + PRUnichar *valueTypeUtf16 = NULL; + PRUnichar *keyDislpayUtf16 = NULL; + PRUnichar *valueDisplayUtf16 = NULL; + + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Type", &keyTypeUtf16); + data->pFuncs->pfnUtf8ToUtf16("sdl", &valueTypeUtf16); + + machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16); + + data->pFuncs->pfnUtf16Free(keyTypeUtf16); + data->pFuncs->pfnUtf16Free(valueTypeUtf16); + + if (sdlDisplay) { + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Display", &keyDislpayUtf16); + data->pFuncs->pfnUtf8ToUtf16(sdlDisplay, &valueDisplayUtf16); + + machine->vtbl->SetExtraData(machine, keyDislpayUtf16, valueDisplayUtf16); + + data->pFuncs->pfnUtf16Free(keyDislpayUtf16); + data->pFuncs->pfnUtf16Free(valueDisplayUtf16); + } + + } else { + /* if all are set then default is gui, with vrdp turned on */ + PRUnichar *keyTypeUtf16 = NULL; + PRUnichar *valueTypeUtf16 = NULL; + PRUnichar *keyDislpayUtf16 = NULL; + PRUnichar *valueDisplayUtf16 = NULL; + + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Type", &keyTypeUtf16); + data->pFuncs->pfnUtf8ToUtf16("gui", &valueTypeUtf16); + + machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16); + + data->pFuncs->pfnUtf16Free(keyTypeUtf16); + data->pFuncs->pfnUtf16Free(valueTypeUtf16); + + if (guiDisplay) { + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Display", &keyDislpayUtf16); + data->pFuncs->pfnUtf8ToUtf16(guiDisplay, &valueDisplayUtf16); + + machine->vtbl->SetExtraData(machine, keyDislpayUtf16, valueDisplayUtf16); + + data->pFuncs->pfnUtf16Free(keyDislpayUtf16); + data->pFuncs->pfnUtf16Free(valueDisplayUtf16); + } } + + VIR_FREE(guiDisplay); + VIR_FREE(sdlDisplay); + } /* Finished:Block to attach the Remote Display to VM */ -#endif { /* Started:Block to attach USB Devices to VM */ if (def->nhostdevs > 0) {

Generally, looks fine apart from the OOM check Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel P. Berrange

6:23 p.m.

New subject: [libvirt] [PATCH 2/3] Support for vrdp/sdl/gui while dumping xml

On Thu, May 07, 2009 at 03:49:38PM +0200, Pritesh Kothari wrote:

...

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 87db6ab..223009a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c + + if (valueDisplayUtf16) { + data->pFuncs->pfnUtf16ToUtf8(valueDisplayUtf16, &valueDisplayUtf8); + data->pFuncs->pfnUtf16Free(valueDisplayUtf16); + + if (strlen(valueDisplayUtf8) <= 0) { + data->pFuncs->pfnUtf8Free(valueDisplayUtf8); + valueDisplayUtf8 = getenv("DISPLAY"); + valueDisplayFree = 0; + } } else { - def->graphics->data.rdp.autoport = 1; + valueDisplayUtf8 = getenv("DISPLAY"); + valueDisplayFree = 0; }

Using getenv() here is not right, since that's running in the context of the app using libvirt. If there's no display available via virtualbox's API, then just leave this attribute blank when generating the XML.

...

- VRDPServer->vtbl->GetNetAddress(VRDPServer, &netAddressUtf16); - if (netAddressUtf16) { - data->pFuncs->pfnUtf16ToUtf8(netAddressUtf16, &netAddressUtf8); - if (STRNEQ(netAddressUtf8, "")) - def->graphics->data.rdp.listenAddr = strdup(netAddressUtf8); - data->pFuncs->pfnUtf16Free(netAddressUtf16); - data->pFuncs->pfnUtf8Free(netAddressUtf8); + if (STREQ(valueTypeUtf8, "sdl")) { + sdlPresent = 1; + sdlDisplay = strdup(valueDisplayUtf8); + totalPresent++; } - VRDPServer->vtbl->GetAuthType(VRDPServer, &authType); - if (authType == VRDPAuthType_External) { - PRUint32 authTimeout = 0; + if (STREQ(valueTypeUtf8, "gui")) { + guiPresent = 1; + guiDisplay = strdup(valueDisplayUtf8); + totalPresent++; + } + if (valueDisplayFree) + data->pFuncs->pfnUtf8Free(valueDisplayUtf8); + } - VRDPServer->vtbl->GetAuthTimeout(VRDPServer, &authTimeout); + if (STREQ(valueTypeUtf8, "vrdp")) + vrdpPresent = 1; - def->graphics->data.rdp.auth = strdup("external"); - def->graphics->data.rdp.authtimeout = authTimeout; - } else if (authType == VRDPAuthType_Guest) { - PRUint32 authTimeout = 0; + data->pFuncs->pfnUtf8Free(valueTypeUtf8); + } - VRDPServer->vtbl->GetAuthTimeout(VRDPServer, &authTimeout); + if ((totalPresent > 0) && (VIR_ALLOC_N(def->graphics, totalPresent) >= 0)) { + if ((guiPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) { + def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP; + def->graphics[def->ngraphics]->data.desktop.display = guiDisplay; + def->ngraphics++; + } - def->graphics->data.rdp.auth = strdup("guest"); - def->graphics->data.rdp.authtimeout = authTimeout; - } + if ((sdlPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) { + def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL; + def->graphics[def->ngraphics]->data.sdl.display = sdlDisplay; + def->ngraphics++; + } + } else if ((vrdpPresent != 1) && (totalPresent == 0) && (VIR_ALLOC_N(def->graphics, 1) >= 0)) { + if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) { + def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP; + def->graphics[def->ngraphics]->data.desktop.display = strdup(getenv("DISPLAY")); + totalPresent++; + def->ngraphics++; + } + } + + machine->vtbl->GetVRDPServer(machine, &VRDPServer); + if (VRDPServer) { + VRDPServer->vtbl->GetEnabled(VRDPServer, &VRDPEnabled); + if (VRDPEnabled) { + totalPresent++; - VRDPServer->vtbl->GetAllowMultiConnection(VRDPServer, &allowMultiConnection); - if (allowMultiConnection) { - def->graphics->data.rdp.multiconnections = 1; - } + if ((VIR_REALLOC_N(def->graphics, totalPresent) >= 0) && + (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) { + PRUint32 VRDPport = 0; + PRUnichar *netAddressUtf16 = NULL; + char *netAddressUtf8 = NULL; + PRBool allowMultiConnection = PR_FALSE; + PRBool reuseSingleConnection = PR_FALSE; - VRDPServer->vtbl->GetReuseSingleConnection(VRDPServer, &reuseSingleConnection); - if (reuseSingleConnection) { - def->graphics->data.rdp.reuseconnection = 1; - } + def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP; - machine->vtbl->GetSessionType(machine, &sessionTypeUtf16); - DEBUG0("Session Type:"); - if (sessionTypeUtf16) { - data->pFuncs->pfnUtf16ToUtf8(sessionTypeUtf16, &sessionTypeUtf8); - DEBUG("Session Type: %s", sessionTypeUtf8); - if (STREQ(sessionTypeUtf8, "vrdp")) { - def->graphics->data.rdp.headless = 1; + VRDPServer->vtbl->GetPort(VRDPServer, &VRDPport); + if (VRDPport) { + def->graphics[def->ngraphics]->data.rdp.port = VRDPport; + } else { + def->graphics[def->ngraphics]->data.rdp.autoport = 1; } - data->pFuncs->pfnUtf16Free(sessionTypeUtf16); - data->pFuncs->pfnUtf8Free(sessionTypeUtf8); + + VRDPServer->vtbl->GetNetAddress(VRDPServer, &netAddressUtf16); + if (netAddressUtf16) { + data->pFuncs->pfnUtf16ToUtf8(netAddressUtf16, &netAddressUtf8); + if (STRNEQ(netAddressUtf8, "")) + def->graphics[def->ngraphics]->data.rdp.listenAddr = strdup(netAddressUtf8); + data->pFuncs->pfnUtf16Free(netAddressUtf16); + data->pFuncs->pfnUtf8Free(netAddressUtf8); + } + + VRDPServer->vtbl->GetAllowMultiConnection(VRDPServer, &allowMultiConnection); + if (allowMultiConnection) { + def->graphics[def->ngraphics]->data.rdp.multiUser = 1; + } + + VRDPServer->vtbl->GetReuseSingleConnection(VRDPServer, &reuseSingleConnection); + if (reuseSingleConnection) { + def->graphics[def->ngraphics]->data.rdp.replaceUser = 1; + } + + def->ngraphics++; } } + VRDPServer->vtbl->nsisupports.Release((nsISupports *)VRDPServer); } - VRDPServer->vtbl->nsisupports.Release((nsISupports *)VRDPServer); } -#endif

There's generally quite a few uses of strdup() here without OOM checking Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel P. Berrange

6:24 p.m.

New subject: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine

On Thu, May 07, 2009 at 03:50:13PM +0200, Pritesh Kothari wrote:

...

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 223009a..97a686f 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2322,20 +2322,12 @@ static int vboxDomainCreate(virDomainPtr dom) { IProgress *progress = NULL; PRUint32 machineCnt = 0; PRUnichar *env = NULL; - const char *display = getenv("DISPLAY"); PRUnichar *sessionType = NULL; char displayutf8[32] = {0}; unsigned char iidl[VIR_UUID_BUFLEN] = {0}; int i, ret = -1; - if (display) { - sprintf(displayutf8, "DISPLAY=%s", display); - data->pFuncs->pfnUtf8ToUtf16(displayutf8, &env); - } - - data->pFuncs->pfnUtf8ToUtf16("gui", &sessionType); - if (!dom->name) { vboxError(dom->conn, VIR_ERR_INTERNAL_ERROR,"%s", "Error while reading the domain name"); @@ -2373,6 +2365,92 @@ static int vboxDomainCreate(virDomainPtr dom) { if ( (state == MachineState_PoweredOff) || (state == MachineState_Saved) || (state == MachineState_Aborted) ) { + int vrdpPresent = 0; + int sdlPresent = 0; + int guiPresent = 0; + char *guiDisplay = NULL; + char *sdlDisplay = NULL; + PRUnichar *keyTypeUtf16 = NULL; + PRUnichar *valueTypeUtf16 = NULL; + char *valueTypeUtf8 = NULL; + PRUnichar *keyDislpayUtf16 = NULL; + PRUnichar *valueDisplayUtf16 = NULL; + char *valueDisplayUtf8 = NULL; + int valueDisplayFree = 1; + + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Type", &keyTypeUtf16); + machine->vtbl->GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16); + data->pFuncs->pfnUtf16Free(keyTypeUtf16); + + if (valueTypeUtf16) { + data->pFuncs->pfnUtf16ToUtf8(valueTypeUtf16, &valueTypeUtf8); + data->pFuncs->pfnUtf16Free(valueTypeUtf16); + + if ( STREQ(valueTypeUtf8, "sdl") || STREQ(valueTypeUtf8, "gui") ) { + + data->pFuncs->pfnUtf8ToUtf16("FRONTEND/Display", &keyDislpayUtf16); + machine->vtbl->GetExtraData(machine, keyDislpayUtf16, &valueDisplayUtf16); + data->pFuncs->pfnUtf16Free(keyDislpayUtf16); + + if (valueDisplayUtf16) { + data->pFuncs->pfnUtf16ToUtf8(valueDisplayUtf16, &valueDisplayUtf8); + data->pFuncs->pfnUtf16Free(valueDisplayUtf16); + + if (strlen(valueDisplayUtf8) <= 0) { + data->pFuncs->pfnUtf8Free(valueDisplayUtf8); + valueDisplayUtf8 = getenv("DISPLAY"); + valueDisplayFree = 0; + } + } else { + valueDisplayUtf8 = getenv("DISPLAY"); + valueDisplayFree = 0; + } + + if (STREQ(valueTypeUtf8, "sdl")) { + sdlPresent = 1; + sdlDisplay = strdup(valueDisplayUtf8); + } + + if (STREQ(valueTypeUtf8, "gui")) { + guiPresent = 1; + guiDisplay = strdup(valueDisplayUtf8); + } + } + + if (STREQ(valueTypeUtf8, "vrdp")) { + vrdpPresent = 1; + } + + data->pFuncs->pfnUtf8Free(valueTypeUtf8); + + } else { + guiPresent = 1; + guiDisplay = strdup(getenv("DISPLAY")); + } + if (valueDisplayFree) + data->pFuncs->pfnUtf8Free(valueDisplayUtf8); + + if (guiPresent) { + sprintf(displayutf8, "DISPLAY=%.24s", guiDisplay); + data->pFuncs->pfnUtf8ToUtf16(displayutf8, &env); + + data->pFuncs->pfnUtf8ToUtf16("gui", &sessionType); + + VIR_FREE(guiDisplay); + } + + if (sdlPresent) { + sprintf(displayutf8, "DISPLAY=%.24s", sdlDisplay); + data->pFuncs->pfnUtf8ToUtf16(displayutf8, &env); + + data->pFuncs->pfnUtf8ToUtf16("sdl", &sessionType); + + VIR_FREE(sdlDisplay); + } + + if (vrdpPresent) { + data->pFuncs->pfnUtf8ToUtf16("vrdp", &sessionType); + }

Same issue about use of 'getenv' & strdup OOM handling here. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Pritesh Kothari

9:56 p.m.

New subject: [libvirt] [PATCH 1/3] Support for vrdp/sdl/gui while defining a machine

...

> [PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine.

...

> + guiDisplay = > strdup(def->graphics[i]->data.desktop.display); + } > + > + if ((def->graphics[i]->type == > VIR_DOMAIN_GRAPHICS_TYPE_SDL) && (sdlPresent == 0)) { + > sdlPresent = 1; > + sdlDisplay = > strdup(def->graphics[i]->data.sdl.display); + } > + } Need to check for OOM failure here.

Done and reposting the patch Regards, Pritesh

Pritesh Kothari

9:56 p.m.

New subject: [libvirt] [PATCH 2/3] Support for vrdp/sdl/gui while dumping xml

...

> - def->graphics->data.rdp.autoport = 1; > + valueDisplayUtf8 = getenv("DISPLAY"); > + valueDisplayFree = 0; > } Using getenv() here is not right, since that's running in the context of the app using libvirt. If there's no display available via virtualbox's API, then just leave this attribute blank when generating the XML. There's generally quite a few uses of strdup() here without OOM checking

Fixed these two and reposting the patch. Regards, Pritesh

Pritesh Kothari

9:56 p.m.

New subject: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine

...

Same issue about use of 'getenv' & strdup OOM handling here.

Fixed these two and reposting the patch. Regards, Pritesh

Daniel P. Berrange

Tuesday, 12 May Tue, 12 May

10:54 p.m.

New subject: [libvirt] [PATCH 1/3] Support for vrdp/sdl/gui while defining a machine

On Mon, May 11, 2009 at 03:56:35PM +0200, Pritesh Kothari wrote:

...

> > [PATCH 1/3]: contains support for vrdp/sdl/gui while defining a machine. + + if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) && (sdlPresent == 0)) { + sdlPresent = 1; + sdlDisplay = strdup(def->graphics[i]->data.sdl.display); + if (sdlDisplay == NULL) { + vboxError(conn, VIR_ERR_SYSTEM_ERROR, "%s", "strdup failed"); + /* just don't go to cleanup yet as it is ok to have + * sdlDisplay as NULL and we check it below if it + * exist and then only use it there + */ + } + }

This wasn't exactly what I anticipated. It intended that it should report the error *and* return as an error to the caller, rather than carrying on. That said, I notice that none of the existing code we have committed for VirtualBox driver deals with OOM on strdup() calls, so this patch is not making things any worse. Thus I think we can commit these patches and fix all the strdup calls in one go, later. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Pritesh Kothari

11:15 p.m.

New subject: [libvirt] [PATCH 1/3] Support for vrdp/sdl/gui while defining a machine

...

This wasn't exactly what I anticipated. It intended that it should report the error *and* return as an error to the caller, rather than carrying on.

hmm.. I thought it more like handling NULL returns from strdup.

...

That said, I notice that none of the existing code we have committed for VirtualBox driver deals with OOM on strdup() calls, so this patch is not making things any worse. Thus I think we can commit these patches and fix all the strdup calls in one go, later.

oops, Sorry this seems to be pretty big slip, yes will try to fix all of them but help is definitely appreciated. Also if strdup() is going to be handle like alloc/VIR_ALLOC way, then i would wait for that and then do the changes. Regards, Pritesh.

Daniel Veillard

Friday, 15 May Fri, 15 May

5:44 p.m.

On Thu, Apr 30, 2009 at 04:15:27PM +0200, Pritesh Kothari wrote:

...

Hi All, As per the discussion earlier on the list I have modified the rdp type and added a new desktop type and posting the patch for same.

Okay, this looks fine, but we also need a description of the format extensions in docs/formatdomain.html.in in the section elementsGraphics Applied and commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Daniel Veillard

9:47 p.m.

New subject: [libvirt] [PATCH 3/3] Support for vrdp/sdl/gui while starting the machine

On Mon, May 11, 2009 at 03:56:50PM +0200, Pritesh Kothari wrote:

...

> > Same issue about use of 'getenv' & strdup OOM handling here. > Fixed these two and reposting the patch.

Okay, I have applied the 3 patches, we still have the strdup() checks missing but it's easier now just as a diff on top of CVS head, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

5632

days inactive

5647

days old

devel@lists.libvirt.org

Manage subscription

14 comments

3 participants

tags (0)

participants (3)

Daniel P. Berrange
Daniel Veillard
Pritesh Kothari

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH 1/1] Support for rdp/desktop types in graphics tag