3:35 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
We try to get the IP address in /domain/devices/graphics/@listen, howerver
for the network type listen address donnot have this parameter, it will
show the address in the /domain/devices/graphics/listen/@address, running
XML like this:
<graphics type='spice' port='5901' autoport='yes'
keymap='en-us'>
<listen type='network' address='192.168.122.1'
network='default'/>
</graphics>
This patch will try to get the IP address in this path
/domain/devices/graphics/listen/@address
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
tools/virsh-domain.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 358d61c..4a9b574 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10024,7 +10024,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
int tmp;
int flags = 0;
bool params = false;
- const char *xpath_fmt =
"string(/domain/devices/graphics[@type='%s']/@%s)";
+ const char *xpath_fmt =
"string(/domain/devices/graphics[@type='%s']/%s)";
virSocketAddr addr;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
@@ -10054,7 +10054,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
continue;
/* Create our XPATH lookup for the current display's port */
- if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "port") < 0)
+ if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@port") < 0)
goto cleanup;
/* Attempt to get the port number for the current graphics scheme */
@@ -10068,7 +10068,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
/* Create our XPATH lookup for TLS Port (automatically skipped
* for unsupported schemes */
- if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "tlsPort") <
0)
+ if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@tlsPort") <
0)
goto cleanup;
/* Attempt to get the TLS port number */
@@ -10081,7 +10081,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
continue;
/* Create our XPATH lookup for the current display's address */
- if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen") < 0)
+ if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen/@address")
< 0)
goto cleanup;
/* Attempt to get the listening addr if set for the current
@@ -10095,7 +10095,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
* care of when getting the XML */
/* Create our XPATH lookup for the password */
- if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "passwd") < 0)
+ if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@passwd") <
0)
goto cleanup;
/* Attempt to get the password */
--
1.8.3.1
2:40 p.m.
On 02/10/2015 04:35 AM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
We try to get the IP address in /domain/devices/graphics/@listen, howerver
for the network type listen address donnot have this parameter, it will
show the address in the /domain/devices/graphics/listen/@address, running
XML like this:
<graphics type='spice' port='5901' autoport='yes'
keymap='en-us'>
<listen type='network' address='192.168.122.1'
network='default'/>
</graphics>
This patch will try to get the IP address in this path
/domain/devices/graphics/listen/@address
That will work when the libvirtd being connected to is 0.9.4 or later,
but earlier versions of libvirt don't have the <listen> subelement;
instead they just have a 'listen' attribute directly inside <graphics>
that contains the address. All newer versions of libvirt are supposed to
populate that from <listen>[0] for backward compatibility.
The real bug here is that the listen attribute in <graphics> isn't being
filled in in the case of type='network' when the domain is active. On
the other hand, fixing the problem there would leave it unfixed for
cases where the client is a new libvirt but the server is running
libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your
patch to check @listen, and if nothing is found there, *then* check
listen/@address. I attached a patch to this mail that I propose
squashing into your patch before pushing. Let me know if it behaves
properly and looks correct.
Beyond that, the server side should still be fixed. I just sent a patch
that does that:
https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html
Between the two patches, we will have fixed the problem for all versions
of server, as long as the client is new enough.
7:40 p.m.
On 02/11/2015 04:40 AM, Laine Stump wrote:
On 02/10/2015 04:35 AM, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1191016
>
> We try to get the IP address in /domain/devices/graphics/@listen, howerver
> for the network type listen address donnot have this parameter, it will
> show the address in the /domain/devices/graphics/listen/@address, running
> XML like this:
>
> <graphics type='spice' port='5901' autoport='yes'
keymap='en-us'>
> <listen type='network' address='192.168.122.1'
network='default'/>
> </graphics>
>
> This patch will try to get the IP address in this path
> /domain/devices/graphics/listen/@address
That will work when the libvirtd being connected to is 0.9.4 or later,
but earlier versions of libvirt don't have the <listen> subelement;
instead they just have a 'listen' attribute directly inside <graphics>
that contains the address. All newer versions of libvirt are supposed to
populate that from <listen>[0] for backward compatibility.
Oh, right, thanks for your catch, i forgot this thing when i wrote this
patch, my patch will cause a issue without the patch you attached when
use new virsh client to connect to old libvirtd(older than 0.9.4).
The real bug here is that the listen attribute in <graphics>
isn't being
filled in in the case of type='network' when the domain is active. On
the other hand, fixing the problem there would leave it unfixed for
cases where the client is a new libvirt but the server is running
libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your
patch to check @listen, and if nothing is found there, *then* check
listen/@address. I attached a patch to this mail that I propose
squashing into your patch before pushing. Let me know if it behaves
properly and looks correct.
I test with the patch you attached, it works well when use new virsh
client connect to new libvirtd and also works well with the old libvirtd
(older than 0.9.4 or later ).
Thanks for your patch.
Beyond that, the server side should still be fixed. I just sent a
patch
that does that:
https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html
Yes, if the server have been fixed, old virsh client command will work
well with new libvirtd.
Between the two patches, we will have fixed the problem for all
versions
of server, as long as the client is new enough.
Thanks for your review and i found you have wrote a patch for the client
side, so seems no need write a new version for this patch :)
Luyao
8:30 p.m.
On 02/11/2015 04:40 AM, Laine Stump wrote:
On 02/10/2015 04:35 AM, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1191016
>
> We try to get the IP address in /domain/devices/graphics/@listen, howerver
> for the network type listen address donnot have this parameter, it will
> show the address in the /domain/devices/graphics/listen/@address, running
> XML like this:
>
> <graphics type='spice' port='5901' autoport='yes'
keymap='en-us'>
> <listen type='network' address='192.168.122.1'
network='default'/>
> </graphics>
>
> This patch will try to get the IP address in this path
> /domain/devices/graphics/listen/@address
That will work when the libvirtd being connected to is 0.9.4 or later,
but earlier versions of libvirt don't have the <listen> subelement;
instead they just have a 'listen' attribute directly inside <graphics>
that contains the address. All newer versions of libvirt are supposed to
populate that from <listen>[0] for backward compatibility.
The real bug here is that the listen attribute in <graphics> isn't being
filled in in the case of type='network' when the domain is active. On
the other hand, fixing the problem there would leave it unfixed for
cases where the client is a new libvirt but the server is running
libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your
patch to check @listen, and if nothing is found there, *then* check
listen/@address. I attached a patch to this mail that I propose
squashing into your patch before pushing. Let me know if it behaves
properly and looks correct.
Beyond that, the server side should still be fixed. I just sent a patch
that does that:
https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html
Between the two patches, we will have fixed the problem for all versions
of server, as long as the client is new enough.
Oh, i forgot the vncdisplay, it should also be fixed, i attached patch
to this mail.
Luyao
3604
days inactive
3605
days old
3 comments
3 participants
participants (3)
-
Laine Stump -
lhuang -
Luyao Huang