Re: [Xen-devel] [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2011/12/5 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> Did patch 1/2 get stuck somewhere? I've not seen it yet.
>
> On Mon, 2011-12-05 at 10:10 +0000, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>> # Date 1323079605 -3600
>> # Node ID c0d51df66b829995c4eb3902b5b9914c710a6c01
>> # Parent Âbc90cfd8dd220d69d09cf94a3d39ff3cef76d021
>> libxl: remove force parameter from libxl_domain_destroy
>>
>> Since a destroy is considered a forced shutdown, there's no point in
>> passing a force parameter. All the occurences of this function have
>> been replaced with the proper syntax.
>
> I'm a little concerned with the change in libxl__destroy_device_model,
> mostly because I don';t know what the expected semantics of a stub dom
> shutdown are. Perhaps it is fine to shoot such a domain in the head
> without previously giving an opportunity to shutdown?

I'm sorry, but I don't know about stubdoms, they are not working under
NetBSD *yet*, I will probably get with this once I finish porting
libxl (or a userspace blktap implementation, that's also quite
outstanding).

>>
>> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>>
>> diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.c
>> --- a/tools/libxl/libxl.c   Mon Dec 05 11:06:23 2011 +0100
>> +++ b/tools/libxl/libxl.c   Mon Dec 05 11:06:45 2011 +0100
>> @@ -718,7 +718,7 @@ int libxl_event_get_disk_eject_info(libx
>> Â Â Âreturn 1;
>> Â}
>>
>> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force)
>> +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>> Â{
>> Â Â Âlibxl__gc gc = LIBXL_INIT_GC(ctx);
>> Â Â Âlibxl_dominfo dominfo;
>> @@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
>>
>> Â Â Â Â Âlibxl__qmp_cleanup(&gc, domid);
>> Â Â Â}
>> - Â Âif (libxl__devices_destroy(&gc, domid, force) < 0)
>> + Â Âif (libxl__devices_destroy(&gc, domid, 1) < 0)
>
> If I'm not missing something this seems to be the only caller of
> libxl__device_destroy. We could keep pushing this change down and remove
> the force param here too which in turns removes a bunch of code from
> libxl__devices_destroy and makes it behave like its name suggests.

I've already created another patch that changes the semantics of
libxl__device_destroy and removes the force flag and all the
unnecessary code. If no one has an objection about this change I will
resend the series later with this patch.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel