Re: [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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [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. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [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:
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
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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
Re: [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
commit 7f23240a48f1f46e0aa4aaaf7f4fde63723e6b98
Author: pk221555 pk221...@krishna.(none)
Date: Mon May 11 14:45:09 2009 +0200
libvirt: added support for vrdp/sdl/gui while defining a machine
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 8e42958..4ef1488 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3041,24 +3041,29 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
}
} /* Finished:Block to attach the Parallel Port to the VM */
-#if 0
{ /* Started:Block to attach the Remote Display to VM */
-if (def-graphics) {
+int vrdpPresent = 0;
+int sdlPresent = 0;
+int guiPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+int i = 0;
+
+for (i = 0; i def-ngraphics; i++) {
IVRDPServer *VRDPServer = NULL;
-/* TODO: include the support for headless stuff
- */
+if ((def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) (vrdpPresent == 0)) {
-if (def-graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) {
+vrdpPresent = 1;
machine-vtbl-GetVRDPServer(machine, VRDPServer);
if (VRDPServer) {
VRDPServer-vtbl-SetEnabled(VRDPServer, PR_TRUE);
DEBUG0(VRDP Support turned ON on port: 3389);
-if (def-graphics-data.rdp.port) {
-VRDPServer-vtbl-SetPort(VRDPServer, def-graphics-data.rdp.port);
-DEBUG(VRDP Port changed to: %d, def-graphics-data.rdp.port);
-} else if (def-graphics-data.rdp.autoport) {
+if (def-graphics[i]-data.rdp.port) {
+VRDPServer-vtbl-SetPort(VRDPServer, def-graphics[i]-data.rdp.port);
+DEBUG(VRDP Port changed to: %d, def-graphics[i]-data.rdp.port);
+} else if (def-graphics[i]-data.rdp.autoport) {
/* Setting the port to 0 will reset its value to
* the default one which is 3389 currently
*/
@@ -3066,37 +3071,22 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
DEBUG0(VRDP Port changed to default, which is 3389 currently);
}
-if (def-graphics-data.rdp.reuseconnection) {
+if (def-graphics[i]-data.rdp.replaceUser) {
VRDPServer-vtbl-SetReuseSingleConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to reuse single connection);
}
-if (def-graphics-data.rdp.multiconnections) {
+if (def-graphics[i]-data.rdp.multiUser) {
VRDPServer-vtbl-SetAllowMultiConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to allow multiple connection);
}
-if (def-graphics-data.rdp.auth) {
-if (STREQ(def-graphics-data.rdp.auth, guest)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_Guest);
-DEBUG0(VRDP authentication method set to Guest);
-} else if (STREQ(def-graphics-data.rdp.auth, external)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_External);
-DEBUG0(VRDP authentication method set to External);
-}
-
-if (def-graphics-data.rdp.authtimeout) {
-VRDPServer-vtbl-SetAuthTimeout(VRDPServer, def-graphics-data.rdp.authtimeout);
-DEBUG(VRDP authentication timeout is set to %llu, def-graphics-data.rdp.authtimeout);
-}
-}
-
-if (def-graphics-data.rdp.listenAddr) {
+if (def-graphics[i]-data.rdp.listenAddr) {
PRUnichar *netAddressUtf16 = NULL;
-
[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
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
@@ -3040,24 +3040,29 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
}
} /* Finished:Block to attach the Parallel Port to the VM */
-#if 0
{ /* Started:Block to attach the Remote Display to VM */
-if (def-graphics) {
+int vrdpPresent = 0;
+int sdlPresent = 0;
+int guiPresent = 0;
+char *guiDisplay = NULL;
+char *sdlDisplay = NULL;
+int i = 0;
+
+for (i = 0; i def-ngraphics; i++) {
IVRDPServer *VRDPServer = NULL;
-/* TODO: include the support for headless stuff
- */
+if ((def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) (vrdpPresent == 0)) {
-if (def-graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) {
+vrdpPresent = 1;
machine-vtbl-GetVRDPServer(machine, VRDPServer);
if (VRDPServer) {
VRDPServer-vtbl-SetEnabled(VRDPServer, PR_TRUE);
DEBUG0(VRDP Support turned ON on port: 3389);
-if (def-graphics-data.rdp.port) {
-VRDPServer-vtbl-SetPort(VRDPServer, def-graphics-data.rdp.port);
-DEBUG(VRDP Port changed to: %d, def-graphics-data.rdp.port);
-} else if (def-graphics-data.rdp.autoport) {
+if (def-graphics[i]-data.rdp.port) {
+VRDPServer-vtbl-SetPort(VRDPServer, def-graphics[i]-data.rdp.port);
+DEBUG(VRDP Port changed to: %d, def-graphics[i]-data.rdp.port);
+} else if (def-graphics[i]-data.rdp.autoport) {
/* Setting the port to 0 will reset its value to
* the default one which is 3389 currently
*/
@@ -3065,37 +3070,22 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
DEBUG0(VRDP Port changed to default, which is 3389 currently);
}
-if (def-graphics-data.rdp.reuseconnection) {
+if (def-graphics[i]-data.rdp.replaceUser) {
VRDPServer-vtbl-SetReuseSingleConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to reuse single connection);
}
-if (def-graphics-data.rdp.multiconnections) {
+if (def-graphics[i]-data.rdp.multiUser) {
VRDPServer-vtbl-SetAllowMultiConnection(VRDPServer, PR_TRUE);
DEBUG0(VRDP set to allow multiple connection);
}
-if (def-graphics-data.rdp.auth) {
-if (STREQ(def-graphics-data.rdp.auth, guest)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_Guest);
-DEBUG0(VRDP authentication method set to Guest);
-} else if (STREQ(def-graphics-data.rdp.auth, external)) {
-VRDPServer-vtbl-SetAuthType(VRDPServer, VRDPAuthType_External);
-DEBUG0(VRDP authentication method set to External);
-}
-
-if (def-graphics-data.rdp.authtimeout) {
-VRDPServer-vtbl-SetAuthTimeout(VRDPServer, def-graphics-data.rdp.authtimeout);
-DEBUG(VRDP authentication timeout is set to %llu, def-graphics-data.rdp.authtimeout);
-}
-}
-
-if (def-graphics-data.rdp.listenAddr) {
+if (def-graphics[i]-data.rdp.listenAddr) {
PRUnichar *netAddressUtf16 = NULL;
-data-pFuncs-pfnUtf8ToUtf16(def-graphics-data.rdp.listenAddr, netAddressUtf16);
+data-pFuncs-pfnUtf8ToUtf16(def-graphics[i]-data.rdp.listenAddr, netAddressUtf16);
VRDPServer-vtbl-SetNetAddress(VRDPServer, netAddressUtf16);
-DEBUG(VRDP listen address is set to: %s, def-graphics-data.rdp.listenAddr);
+DEBUG(VRDP listen
