[PATCH] gdbsx: use right path for privcmd

Manuel Bouyer posted 1 patch 3 years, 3 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210112181242.1570-8-bouyer@antioche.eu.org
tools/debugger/gdbsx/xg/xg_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] gdbsx: use right path for privcmd
Posted by Manuel Bouyer 3 years, 3 months ago
From: Manuel Bouyer <bouyer@netbsd.org>

On NetBSD the privcmd interface node is /kern/xen/privcmd

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/debugger/gdbsx/xg/xg_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
index ce95648e7e..83a009c195 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -130,11 +130,11 @@ xg_init()
     int flags, saved_errno;
 
     XGTRC("E\n");
-    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
-        if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
-            perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
-            return -1;
-        }
+    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1 &&
+        (_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1 &&
+	(_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
+        perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or /kern/xen/privcmd\n");
+        return -1;
     }
     /* Although we return the file handle as the 'xc handle' the API
      * does not specify / guarentee that this integer is in fact
-- 
2.29.2


Re: [PATCH] gdbsx: use right path for privcmd
Posted by Roger Pau Monné 3 years, 3 months ago
On Tue, Jan 12, 2021 at 07:12:28PM +0100, Manuel Bouyer wrote:
> From: Manuel Bouyer <bouyer@netbsd.org>
> 
> On NetBSD the privcmd interface node is /kern/xen/privcmd
> 
> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> ---
>  tools/debugger/gdbsx/xg/xg_main.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
> index ce95648e7e..83a009c195 100644
> --- a/tools/debugger/gdbsx/xg/xg_main.c
> +++ b/tools/debugger/gdbsx/xg/xg_main.c
> @@ -130,11 +130,11 @@ xg_init()
>      int flags, saved_errno;
>  
>      XGTRC("E\n");
> -    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
> -        if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
> -            perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
> -            return -1;
> -        }
> +    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1 &&
> +        (_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1 &&
> +	(_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {

Nit: hard tab instead of spaces.

> +        perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or /kern/xen/privcmd\n");

I would have split the line, so:

        perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or "
	       "/kern/xen/privcmd\n");

If you can resend with those fixed please add:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH] gdbsx: use right path for privcmd
Posted by Andrew Cooper 3 years, 3 months ago
On 18/01/2021 18:03, Roger Pau Monné wrote:
> On Tue, Jan 12, 2021 at 07:12:28PM +0100, Manuel Bouyer wrote:
>> From: Manuel Bouyer <bouyer@netbsd.org>
>>
>> On NetBSD the privcmd interface node is /kern/xen/privcmd
>>
>> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
>> ---
>>  tools/debugger/gdbsx/xg/xg_main.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
>> index ce95648e7e..83a009c195 100644
>> --- a/tools/debugger/gdbsx/xg/xg_main.c
>> +++ b/tools/debugger/gdbsx/xg/xg_main.c
>> @@ -130,11 +130,11 @@ xg_init()
>>      int flags, saved_errno;
>>  
>>      XGTRC("E\n");
>> -    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
>> -        if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
>> -            perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
>> -            return -1;
>> -        }
>> +    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1 &&
>> +        (_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1 &&
>> +	(_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
> Nit: hard tab instead of spaces.
>
>> +        perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or /kern/xen/privcmd\n");
> I would have split the line, so:
>
>         perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or "
> 	       "/kern/xen/privcmd\n");
>
> If you can resend with those fixed please add:
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I'd shorten it to just "Failed to open privcmd".  I can fix on commit if
you're happy.

~Andrew

P.S. Part of me doesn't want to know why we're opencoding libxencall here...

Re: [PATCH] gdbsx: use right path for privcmd
Posted by Manuel Bouyer 3 years, 3 months ago
On Mon, Jan 18, 2021 at 06:45:42PM +0000, Andrew Cooper wrote:
> On 18/01/2021 18:03, Roger Pau Monné wrote:
> > On Tue, Jan 12, 2021 at 07:12:28PM +0100, Manuel Bouyer wrote:
> >> From: Manuel Bouyer <bouyer@netbsd.org>
> >>
> >> On NetBSD the privcmd interface node is /kern/xen/privcmd
> >>
> >> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
> >> ---
> >>  tools/debugger/gdbsx/xg/xg_main.c | 10 +++++-----
> >>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
> >> index ce95648e7e..83a009c195 100644
> >> --- a/tools/debugger/gdbsx/xg/xg_main.c
> >> +++ b/tools/debugger/gdbsx/xg/xg_main.c
> >> @@ -130,11 +130,11 @@ xg_init()
> >>      int flags, saved_errno;
> >>  
> >>      XGTRC("E\n");
> >> -    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
> >> -        if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
> >> -            perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
> >> -            return -1;
> >> -        }
> >> +    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1 &&
> >> +        (_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1 &&
> >> +	(_dom0_fd=open("/kern/xen/privcmd", O_RDWR)) == -1) {
> > Nit: hard tab instead of spaces.
> >
> >> +        perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or /kern/xen/privcmd\n");
> > I would have split the line, so:
> >
> >         perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd or "
> > 	       "/kern/xen/privcmd\n");
> >
> > If you can resend with those fixed please add:
> >
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> I'd shorten it to just "Failed to open privcmd".  I can fix on commit if
> you're happy.

fine with me, thanks !

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--