Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
On Thu, Sep 22, 2016 at 11:36:40AM +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH v1 3/3] xen-livepatch: If hypervisor is not
> compiled with Livepatching"):
> > Konrad Rzeszutek Wilk writes ("Re: [PATCH v1 3/3] xen-livepatch: If
> > hypervisor is not compiled with Livepatching"):
> > > Why would fprintf mess up 'errno' ?
> >
> > Any libc function is entitled to set errno, if it fails.
>
> And, I forget the exact rules, but I think most are allowed to trash
> it even if they succeed. I just remember that errno needs to be saved
> across libc calls, if the value is important.
>
> In practice, for example, the first fprintf on a particular FILE* can
> end up setting errno to ENOTTY as the libc calls isatty() to decide on
> the buffering mode.
Aaah! Thanks!
>
> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
On 09/22/2016 11:12 AM, Konrad Rzeszutek Wilk wrote:
On Thu, Sep 22, 2016 at 08:25:33AM +0100, Ross Lagerwall wrote:
On 09/21/2016 08:20 PM, Konrad Rzeszutek Wilk wrote:
. print a better error code.
Reported-by: Andrew Cooper
Signed-off-by: Konrad Rzeszutek Wilk
---
tools/misc/xen-livepatch.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2de04c0..f11e71f07f 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -100,8 +100,11 @@ static int list_func(int argc, char *argv[])
rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done,
&left);
if ( rc )
{
-fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
-idx, left, errno, strerror(errno));
+if ( errno == ENOSYS )
+fprintf(stderr, "Hypervisor compiled without Xen
Livepatching!\n");
+else
+fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
+idx, left, errno, strerror(errno));
break;
}
if ( !idx )
You should save errno since you have an fprintf before you check it.
I am missing something.
Why would fprintf mess up 'errno' ?
If the first fprintf fails for whatever reason (e.g. ENOSPC), then it
will set errno and the subsequent check of errno afterwards will serve
no purpose.
I have a patch to fix this issue for the rest of xen-livepatch.c, and
will send it out shortly.
--
Ross Lagerwall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
Ian Jackson writes ("Re: [PATCH v1 3/3] xen-livepatch: If hypervisor is not
compiled with Livepatching"):
> Konrad Rzeszutek Wilk writes ("Re: [PATCH v1 3/3] xen-livepatch: If
> hypervisor is not compiled with Livepatching"):
> > Why would fprintf mess up 'errno' ?
>
> Any libc function is entitled to set errno, if it fails.
And, I forget the exact rules, but I think most are allowed to trash
it even if they succeed. I just remember that errno needs to be saved
across libc calls, if the value is important.
In practice, for example, the first fprintf on a particular FILE* can
end up setting errno to ENOTTY as the libc calls isatty() to decide on
the buffering mode.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
Konrad Rzeszutek Wilk writes ("Re: [PATCH v1 3/3] xen-livepatch: If hypervisor
is not compiled with Livepatching"):
> On Thu, Sep 22, 2016 at 08:25:33AM +0100, Ross Lagerwall wrote:
> > You should save errno since you have an fprintf before you check it.
>
> I am missing something.
>
> Why would fprintf mess up 'errno' ?
Any libc function is entitled to set errno, if it fails.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
On Thu, Sep 22, 2016 at 08:25:33AM +0100, Ross Lagerwall wrote:
> On 09/21/2016 08:20 PM, Konrad Rzeszutek Wilk wrote:
> > . print a better error code.
> >
> > Reported-by: Andrew Cooper
> > Signed-off-by: Konrad Rzeszutek Wilk
> > ---
> > tools/misc/xen-livepatch.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
> > index 2de04c0..f11e71f07f 100644
> > --- a/tools/misc/xen-livepatch.c
> > +++ b/tools/misc/xen-livepatch.c
> > @@ -100,8 +100,11 @@ static int list_func(int argc, char *argv[])
> > rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done,
> > &left);
> > if ( rc )
> > {
> > -fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
> > -idx, left, errno, strerror(errno));
> > +if ( errno == ENOSYS )
> > +fprintf(stderr, "Hypervisor compiled without Xen
> > Livepatching!\n");
> > +else
> > +fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
> > +idx, left, errno, strerror(errno));
> > break;
> > }
> > if ( !idx )
> >
>
> You should save errno since you have an fprintf before you check it.
I am missing something.
Why would fprintf mess up 'errno' ?
>
> Also, it would be good to do the same check for the first xc_livepatch_get
> in action_func() and for the xc_livepatch_upload in upload_func.
>
> --
> Ross Lagerwall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
On 09/21/2016 08:20 PM, Konrad Rzeszutek Wilk wrote:
. print a better error code.
Reported-by: Andrew Cooper
Signed-off-by: Konrad Rzeszutek Wilk
---
tools/misc/xen-livepatch.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2de04c0..f11e71f07f 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -100,8 +100,11 @@ static int list_func(int argc, char *argv[])
rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done,
&left);
if ( rc )
{
-fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
-idx, left, errno, strerror(errno));
+if ( errno == ENOSYS )
+fprintf(stderr, "Hypervisor compiled without Xen
Livepatching!\n");
+else
+fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
+idx, left, errno, strerror(errno));
break;
}
if ( !idx )
You should save errno since you have an fprintf before you check it.
Also, it would be good to do the same check for the first
xc_livepatch_get in action_func() and for the xc_livepatch_upload in
upload_func.
--
Ross Lagerwall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v1 3/3] xen-livepatch: If hypervisor is not compiled with Livepatching
. print a better error code.
Reported-by: Andrew Cooper
Signed-off-by: Konrad Rzeszutek Wilk
---
tools/misc/xen-livepatch.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2de04c0..f11e71f07f 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -100,8 +100,11 @@ static int list_func(int argc, char *argv[])
rc = xc_livepatch_list(xch, MAX_LEN, idx, info, name, len, &done,
&left);
if ( rc )
{
-fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
-idx, left, errno, strerror(errno));
+if ( errno == ENOSYS )
+fprintf(stderr, "Hypervisor compiled without Xen
Livepatching!\n");
+else
+fprintf(stderr, "Failed to list %d/%d: %d(%s)!\n",
+idx, left, errno, strerror(errno));
break;
}
if ( !idx )
--
2.4.11
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
