* [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
@ 2012-05-24 15:37 Dario Faggioli
2012-05-25 9:44 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Dario Faggioli @ 2012-05-24 15:37 UTC (permalink / raw)
To: xen-devel; +Cc: Christoph Egger, Ian Jackson, Ian Campbell, Roger Pau Monne
To avoid recent gcc complaining about:
libxl.c: In function ‘libxl_primary_console_exec’:
libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
Adjust code pieces where -Wswitch makes it claim that
LIBXL_DOMAIN_TYPE_INVALID is not handled.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1230,7 +1230,7 @@ int libxl_primary_console_exec(libxl_ctx
case LIBXL_DOMAIN_TYPE_PV:
rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
break;
- case -1:
+ case LIBXL_DOMAIN_TYPE_INVALID:
LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
rc = ERROR_FAIL;
break;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -257,6 +257,10 @@ static char ** libxl__build_device_model
for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra_hvm[i]);
break;
+ case LIBXL_DOMAIN_TYPE_INVALID:
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
+ flexarray_free(dm_args);
+ return NULL;
}
flexarray_append(dm_args, NULL);
return (char **) flexarray_contents(dm_args);
@@ -505,6 +509,10 @@ static char ** libxl__build_device_model
for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra_hvm[i]);
break;
+ case LIBXL_DOMAIN_TYPE_INVALID:
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
+ flexarray_free(dm_args);
+ return NULL;
}
ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
if (ret != 1)
- return -1;
+ return LIBXL_DOMAIN_TYPE_INVALID;
if (info.domain != domid)
- return -1;
+ return LIBXL_DOMAIN_TYPE_INVALID;
if (info.flags & XEN_DOMINF_hvm_guest)
return LIBXL_DOMAIN_TYPE_HVM;
else
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
#
libxl_domain_type = Enumeration("domain_type", [
+ (-1, "INVALID"),
(1, "HVM"),
(2, "PV"),
])
@@ -315,6 +316,7 @@ libxl_domain_build_info = Struct("domain
# Use host's E820 for PCI passthrough.
("e820_host", libxl_defbool),
])),
+ ("invalid", Struct(None, [])),
], keyvar_init_val = "-1")),
], dir=DIR_IN
)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-24 15:37 [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID Dario Faggioli
@ 2012-05-25 9:44 ` Ian Campbell
2012-05-25 10:26 ` Dario Faggioli
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-05-25 9:44 UTC (permalink / raw)
To: Dario Faggioli
Cc: Christoph Egger, Roger Pau Monne, Ian Jackson,
xen-devel@lists.xen.org
On Thu, 2012-05-24 at 16:37 +0100, Dario Faggioli wrote:
> To avoid recent gcc complaining about:
> libxl.c: In function ‘libxl_primary_console_exec’:
> libxl.c:1233:9: error: case value ‘4294967295’ not in enumerated type ‘libxl_domain_type’ [-Werror=switch]
>
> Adjust code pieces where -Wswitch makes it claim that
> LIBXL_DOMAIN_TYPE_INVALID is not handled.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1230,7 +1230,7 @@ int libxl_primary_console_exec(libxl_ctx
> case LIBXL_DOMAIN_TYPE_PV:
> rc = libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV);
> break;
> - case -1:
> + case LIBXL_DOMAIN_TYPE_INVALID:
> LOG(ERROR,"unable to get domain type for domid=%"PRIu32,domid_vm);
> rc = ERROR_FAIL;
> break;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -257,6 +257,10 @@ static char ** libxl__build_device_model
> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> flexarray_append(dm_args, b_info->extra_hvm[i]);
> break;
> + case LIBXL_DOMAIN_TYPE_INVALID:
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> + flexarray_free(dm_args);
> + return NULL;
In some cases, like here, the code is entitled to assume that type is
either PV or HVM, due to the checks in
libxl__domain_build_info_setdefault. I think if we see an invalid here
then that is an abort() worthy event.
There are a bunch of places where we look a b_info->type with if
statements instead of switch. Plain ifs are ok, but it might be worth
checking the ones with an else clause (not else if) too? I suspect in
many cases they are entitled that !HVM == PV due to the setdefault
thing.
> }
> flexarray_append(dm_args, NULL);
> return (char **) flexarray_contents(dm_args);
> @@ -505,6 +509,10 @@ static char ** libxl__build_device_model
> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> flexarray_append(dm_args, b_info->extra_hvm[i]);
> break;
> + case LIBXL_DOMAIN_TYPE_INVALID:
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> + flexarray_free(dm_args);
> + return NULL;
abort() here too.
> }
>
> ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -33,9 +33,9 @@ libxl_domain_type libxl__domain_type(lib
>
> ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
> if (ret != 1)
> - return -1;
> + return LIBXL_DOMAIN_TYPE_INVALID;
> if (info.domain != domid)
> - return -1;
> + return LIBXL_DOMAIN_TYPE_INVALID;
> if (info.flags & XEN_DOMINF_hvm_guest)
> return LIBXL_DOMAIN_TYPE_HVM;
> else
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -30,6 +30,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
> #
>
> libxl_domain_type = Enumeration("domain_type", [
> + (-1, "INVALID"),
> (1, "HVM"),
> (2, "PV"),
> ])
> @@ -315,6 +316,7 @@ libxl_domain_build_info = Struct("domain
> # Use host's E820 for PCI passthrough.
> ("e820_host", libxl_defbool),
> ])),
> + ("invalid", Struct(None, [])),
> ], keyvar_init_val = "-1")),
> ], dir=DIR_IN
> )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 9:44 ` Ian Campbell
@ 2012-05-25 10:26 ` Dario Faggioli
2012-05-25 10:36 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Dario Faggioli @ 2012-05-25 10:26 UTC (permalink / raw)
To: Ian Campbell
Cc: Christoph Egger, Roger Pau Monne, Ian Jackson,
xen-devel@lists.xen.org
[-- Attachment #1.1: Type: text/plain, Size: 4336 bytes --]
On Fri, 2012-05-25 at 10:44 +0100, Ian Campbell wrote:
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -257,6 +257,10 @@ static char ** libxl__build_device_model
> > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> > flexarray_append(dm_args, b_info->extra_hvm[i]);
> > break;
> > + case LIBXL_DOMAIN_TYPE_INVALID:
> > + LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> > + flexarray_free(dm_args);
> > + return NULL;
>
> In some cases, like here, the code is entitled to assume that type is
> either PV or HVM, due to the checks in
> libxl__domain_build_info_setdefault. I think if we see an invalid here
> then that is an abort() worthy event.
>
As you wish, although it seems to me that this case above is the only
possible situation where we are returning NULL from that function.
Nevertheless, a NULL return value is handled and considered (and
propagated) as error by the caller. That's why, when Roger suggested
going this way (i.e., retuning NULL), I liked the idea very much and
went for it.
That being said, I'm very very very few confident with these code paths,
so I'm just thought it might worth pointing the above out, but I'll
definitely cope with your request if you really thing this is they
correct thing o do.
> There are a bunch of places where we look a b_info->type with if
> statements instead of switch. Plain ifs are ok, but it might be worth
> checking the ones with an else clause (not else if) too? I suspect in
> many cases they are entitled that !HVM == PV due to the setdefault
> thing.
>
Right, and many of them I can take care of. Perhaps the problem is there
are some places where the event of libxl__domain_type "failing" (i.e.,
returning something different from HVM or PV) is somewhat considered
impossible, or at least neglected, like this one here:
int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
uint32_t domid, int fd)
{
GC_INIT(ctx);
libxl_domain_type type = libxl__domain_type(gc, domid);
int live = info != NULL && info->flags & XL_SUSPEND_LIVE;
int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
int rc = 0;
rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
/* No Remus */ NULL);
if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
rc = libxl__domain_save_device_model(gc, domid, fd);
GC_FREE;
return rc;
}
Of course that can be right because of your argument about _sefdefault,
but I'm not sure how to make sure of that and what to do about them...
Thoughts?
On a related note, re what we where discussing yesterday on IRC about
putting a 'default' clause on those switches, there already is some
heterogeneity here. For example:
int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
{
...
switch (libxl__domain_type(gc, domid)) {
case LIBXL_DOMAIN_TYPE_HVM:
...
break;
case LIBXL_DOMAIN_TYPE_PV:
...
break;
default:
abort();
}
...
}
or:
int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm)
{
...
else {
switch (libxl__domain_type(gc, domid_vm)) {
case LIBXL_DOMAIN_TYPE_HVM:
...
break;
case LIBXL_DOMAIN_TYPE_PV:
...
break;
case LIBXL_DOMAIN_TYPE_INVALID:
...
break;
default:
abort();
}
...
}
Should we leave it like this? Is it worth/reasonable to convert all the
'default:' into 'case LIBXL_DOMAIN_TYPE_INVALID:' (if yes, what do we do
with the places that have both? :O) ? Or perhaps vice versa?
If we can gather consensus on an alternative, I can go ahead and put it
down all over the places...
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 10:26 ` Dario Faggioli
@ 2012-05-25 10:36 ` Ian Campbell
2012-05-25 11:03 ` Ian Jackson
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-05-25 10:36 UTC (permalink / raw)
To: Dario Faggioli
Cc: Christoph Egger, Roger Pau Monne, Ian Jackson,
xen-devel@lists.xen.org
On Fri, 2012-05-25 at 11:26 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-25 at 10:44 +0100, Ian Campbell wrote:
> > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > --- a/tools/libxl/libxl_dm.c
> > > +++ b/tools/libxl/libxl_dm.c
> > > @@ -257,6 +257,10 @@ static char ** libxl__build_device_model
> > > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> > > flexarray_append(dm_args, b_info->extra_hvm[i]);
> > > break;
> > > + case LIBXL_DOMAIN_TYPE_INVALID:
> > > + LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "invalid domain type");
> > > + flexarray_free(dm_args);
> > > + return NULL;
> >
> > In some cases, like here, the code is entitled to assume that type is
> > either PV or HVM, due to the checks in
> > libxl__domain_build_info_setdefault. I think if we see an invalid here
> > then that is an abort() worthy event.
> >
> As you wish, although it seems to me that this case above is the only
> possible situation where we are returning NULL from that function.
> Nevertheless, a NULL return value is handled and considered (and
> propagated) as error by the caller. That's why, when Roger suggested
> going this way (i.e., retuning NULL), I liked the idea very much and
> went for it.
>
> That being said, I'm very very very few confident with these code paths,
> so I'm just thought it might worth pointing the above out, but I'll
> definitely cope with your request if you really thing this is they
> correct thing o do.
It would be a bug, similar to an assertion failure, to get to this point
with b_info->type not equal to either PV or HVM. This comment about
setdefault in libxl_internal.h explains why:
* Idempotently sets any members of "p" which is currently set to
* a special value indicating that the defaults should be used
* (per libxl_<type>_init) to a specific value.
*
* All libxl API functions are expected to have arranged for this
* to be called before using any values within these structures.
so having arranged to call that function at the right time we can assume
that type is a sensible value, and indeed setdefault makes this the
case.
> > There are a bunch of places where we look a b_info->type with if
> > statements instead of switch. Plain ifs are ok, but it might be worth
> > checking the ones with an else clause (not else if) too? I suspect in
> > many cases they are entitled that !HVM == PV due to the setdefault
> > thing.
> >
>
> Right, and many of them I can take care of. Perhaps the problem is there
> are some places where the event of libxl__domain_type "failing" (i.e.,
> returning something different from HVM or PV) is somewhat considered
> impossible, or at least neglected, like this one here:
>
> int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
> uint32_t domid, int fd)
> {
> GC_INIT(ctx);
> libxl_domain_type type = libxl__domain_type(gc, domid);
> int live = info != NULL && info->flags & XL_SUSPEND_LIVE;
> int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
> int rc = 0;
>
> rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
> /* No Remus */ NULL);
>
> if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
> rc = libxl__domain_save_device_model(gc, domid, fd);
> GC_FREE;
> return rc;
> }
>
>
> Of course that can be right because of your argument about _sefdefault,
> but I'm not sure how to make sure of that and what to do about them...
> Thoughts?
This one is OK, it doesn't have an else case...
>
>
> On a related note, re what we where discussing yesterday on IRC about
> putting a 'default' clause on those switches, there already is some
> heterogeneity here. For example:
>
>
> int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
> {
> ...
> switch (libxl__domain_type(gc, domid)) {
> case LIBXL_DOMAIN_TYPE_HVM:
> ...
> break;
> case LIBXL_DOMAIN_TYPE_PV:
> ...
> break;
> default:
> abort();
This seems like it needs a TYPE_INVALID, since libxl__domain_type could
legitimately return it.
> }
> ...
> }
>
> or:
>
> int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm)
> {
> ...
> else {
> switch (libxl__domain_type(gc, domid_vm)) {
> case LIBXL_DOMAIN_TYPE_HVM:
> ...
> break;
> case LIBXL_DOMAIN_TYPE_PV:
> ...
> break;
> case LIBXL_DOMAIN_TYPE_INVALID:
> ...
> break;
> default:
> abort();
> }
> ...
> }
>
> Should we leave it like this? Is it worth/reasonable to convert all the
> 'default:' into 'case LIBXL_DOMAIN_TYPE_INVALID:' (if yes, what do we do
> with the places that have both? :O) ? Or perhaps vice versa?
>
> If we can gather consensus on an alternative, I can go ahead and put it
> down all over the places...
>
> Thanks and Regards,
> Dario
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 10:36 ` Ian Campbell
@ 2012-05-25 11:03 ` Ian Jackson
2012-05-25 13:10 ` Dario Faggioli
0 siblings, 1 reply; 9+ messages in thread
From: Ian Jackson @ 2012-05-25 11:03 UTC (permalink / raw)
To: Ian Campbell
Cc: Christoph Egger, Roger Pau Monne, Dario Faggioli,
xen-devel@lists.xen.org
Ian Campbell writes ("Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
> so having arranged to call that function at the right time we can assume
> that type is a sensible value, and indeed setdefault makes this the
> case.
Right.
The other situation where we can get _INVALID is if libxl__domain_type
fails, which it can do.
I think this should be handled by having places which call
libxl__domain_type abandon operation and return an error if the
libxl__domain_type fails.
If this is done, then general variables, parameters, etc. within libxl
which are supposed to contain a libxl_domain_type will never contain
_INVALID.
Ian.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 11:03 ` Ian Jackson
@ 2012-05-25 13:10 ` Dario Faggioli
2012-05-25 14:00 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Dario Faggioli @ 2012-05-25 13:10 UTC (permalink / raw)
To: Ian Jackson
Cc: Christoph Egger, Roger Pau Monne, Ian Campbell,
xen-devel@lists.xen.org
[-- Attachment #1.1: Type: text/plain, Size: 1324 bytes --]
On Fri, 2012-05-25 at 12:03 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
> > so having arranged to call that function at the right time we can assume
> > that type is a sensible value, and indeed setdefault makes this the
> > case.
>
> Right.
>
Ok.
> The other situation where we can get _INVALID is if libxl__domain_type
> fails, which it can do.
>
> I think this should be handled by having places which call
> libxl__domain_type abandon operation and return an error if the
> libxl__domain_type fails.
>
> If this is done, then general variables, parameters, etc. within libxl
> which are supposed to contain a libxl_domain_type will never contain
> _INVALID.
>
I like this. I'll chase each call to that function and have the calle
failing if a DOMAIN_TYPE_INVALID is returned. Then, if I go this way,
can I also nuke both the 'case DOMAIN_TYPE_INVALID' _and_ the default
clauses from everywhere? I seem to think I could...
Thanks and Regards,
Daio
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 13:10 ` Dario Faggioli
@ 2012-05-25 14:00 ` Ian Campbell
2012-06-04 13:11 ` Christoph Egger
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2012-05-25 14:00 UTC (permalink / raw)
To: Dario Faggioli
Cc: Christoph Egger, Roger Pau Monne, Ian Jackson,
xen-devel@lists.xen.org
On Fri, 2012-05-25 at 14:10 +0100, Dario Faggioli wrote:
> On Fri, 2012-05-25 at 12:03 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
> > > so having arranged to call that function at the right time we can assume
> > > that type is a sensible value, and indeed setdefault makes this the
> > > case.
> >
> > Right.
> >
> Ok.
>
> > The other situation where we can get _INVALID is if libxl__domain_type
> > fails, which it can do.
> >
> > I think this should be handled by having places which call
> > libxl__domain_type abandon operation and return an error if the
> > libxl__domain_type fails.
> >
> > If this is done, then general variables, parameters, etc. within libxl
> > which are supposed to contain a libxl_domain_type will never contain
> > _INVALID.
> >
> I like this. I'll chase each call to that function and have the calle
> failing if a DOMAIN_TYPE_INVALID is returned. Then, if I go this way,
> can I also nuke both the 'case DOMAIN_TYPE_INVALID' _and_ the default
> clauses from everywhere? I seem to think I could...
iff the compiler is smart enough to realise that in the type == INVALID
case you have returned already before reaching the switch statement,
otherwise you will need to have "case INVALID: abort()".
>
> Thanks and Regards,
> Daio
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-05-25 14:00 ` Ian Campbell
@ 2012-06-04 13:11 ` Christoph Egger
2012-06-04 14:03 ` Dario Faggioli
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Egger @ 2012-06-04 13:11 UTC (permalink / raw)
To: Ian Campbell
Cc: Ian Jackson, Roger Pau Monne, Dario Faggioli,
xen-devel@lists.xen.org
On 05/25/12 16:00, Ian Campbell wrote:
> On Fri, 2012-05-25 at 14:10 +0100, Dario Faggioli wrote:
>> On Fri, 2012-05-25 at 12:03 +0100, Ian Jackson wrote:
>>> Ian Campbell writes ("Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID"):
>>>> so having arranged to call that function at the right time we can assume
>>>> that type is a sensible value, and indeed setdefault makes this the
>>>> case.
>>>
>>> Right.
>>>
>> Ok.
>>
>>> The other situation where we can get _INVALID is if libxl__domain_type
>>> fails, which it can do.
>>>
>>> I think this should be handled by having places which call
>>> libxl__domain_type abandon operation and return an error if the
>>> libxl__domain_type fails.
>>>
>>> If this is done, then general variables, parameters, etc. within libxl
>>> which are supposed to contain a libxl_domain_type will never contain
>>> _INVALID.
>>>
>> I like this. I'll chase each call to that function and have the calle
>> failing if a DOMAIN_TYPE_INVALID is returned. Then, if I go this way,
>> can I also nuke both the 'case DOMAIN_TYPE_INVALID' _and_ the default
>> clauses from everywhere? I seem to think I could...
>
> iff the compiler is smart enough to realise that in the type == INVALID
> case you have returned already before reaching the switch statement,
> otherwise you will need to have "case INVALID: abort()".
What is latest status of this patch? When will it go upstream?
>
>>
>> Thanks and Regards,
>> Daio
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID
2012-06-04 13:11 ` Christoph Egger
@ 2012-06-04 14:03 ` Dario Faggioli
0 siblings, 0 replies; 9+ messages in thread
From: Dario Faggioli @ 2012-06-04 14:03 UTC (permalink / raw)
To: Christoph Egger
Cc: Roger Pau Monne, Ian Jackson, Ian Campbell,
xen-devel@lists.xen.org
[-- Attachment #1.1: Type: text/plain, Size: 530 bytes --]
On Mon, 2012-06-04 at 15:11 +0200, Christoph Egger wrote:
> What is latest status of this patch? When will it go upstream?
>
I'm resending a new version of it in a very short while... Let's see if
I manage in intercepting everyone's taste! :-)
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-06-04 14:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-24 15:37 [PATCH v4] libxl: introduce LIBXL_DOMAIN_TYPE_INVALID Dario Faggioli
2012-05-25 9:44 ` Ian Campbell
2012-05-25 10:26 ` Dario Faggioli
2012-05-25 10:36 ` Ian Campbell
2012-05-25 11:03 ` Ian Jackson
2012-05-25 13:10 ` Dario Faggioli
2012-05-25 14:00 ` Ian Campbell
2012-06-04 13:11 ` Christoph Egger
2012-06-04 14:03 ` Dario Faggioli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).