On Wed, 2015-07-22 at 17:18 +0800, Chen, Tiejun wrote:
Thanks for your clarification to me.
The solution to this is to be systematic in how you handle the
email
based review of a series, not to add a further to the reviewer by
using
IRC as well.
For example, here is how I
Thanks for your clarification to me.
The solution to this is to be systematic in how you handle the email
based review of a series, not to add a further to the reviewer by using
IRC as well.
For example, here is how I handle review of a series I am working on:
I keep all the replies to a
Chen, Tiejun writes (Re: [Xen-devel] [v10][PATCH 11/16] tools/libxl: detect
and avoid conflicts with RDM):
I then go through the comments one by one and either:
* make the _complete_ code change, including adding the Changes
in vN bit to the commit log and delete
On Wed, 2015-07-22 at 08:33 +0800, Chen, Tiejun wrote:
On 2015/7/21 23:57, Ian Jackson wrote:
It's true that I didn't comment on the frat that you had half-done one
of the things I had requested. It is of course a waste of my time to
be constantly re-reviewing half-done changes.
Next
Just to your example,
libxl_domain_config cfg;
cfg.stuff = blah;
cfg.rdm.strategy = HOST;
libxl_domain_create_new(cfg, domid);
libxl_domain_destroy(domid);
Here looks you mean d_config-rdms would be changed, right? Currently
this shouldn't be allowed. But I think we need to
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
+static void
+add_rdm_entry(libxl__gc *gc, libxl_domain_config *d_config,
+ uint64_t rdm_start, uint64_t rdm_size, int rdm_policy)
+{
+d_config-num_rdms++;
+d_config-rdms =
On 2015/7/21 23:09, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
+static void
+add_rdm_entry(libxl__gc *gc, libxl_domain_config *d_config,
+ uint64_t rdm_start, uint64_t rdm_size, int rdm_policy)
+{
+
Ian Jackson writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
Sorry, I just ignore the line in brackets since I always think this kind
of thing is often not a big
On 2015/7/21 23:57, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
Sorry, I just ignore the line in brackets since I always think this kind
of thing is often not a big deal, and next time I should pay more
attention to the ().
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
[Ian Jackson:]
That is, an address would be reserved if it was reserved in any of the
rdm regions implied by the config.
Are you saying this point?
The union of two sets A and B is the set of
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
Indeed, I'm not a fan to Xen tools so I can't picture what this real
scenario would happen. So if I'm misunderstanding what you mean, just
please correct me. Or if you still think its hard to
I wrote:
If the domain configuration has rdms and num_rdms already set, then
the strategy should presumably be ignored. (Passing the same domain
configuration struct to libxl_domain_create_new, after destroying the
domain, ought to work, even after the first call has modified it.)
But even
I hope the following can address all comments below:
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2f8e590..a4bd2a1 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -407,7 +407,7 @@ int libxl__domain_build(libxl__gc *gc,
switch
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
I would add this check at the beginning of this function:
assert(!d_config-num_rdms !d_config-rdms).
As Ian Campbell and I have explained, that is not OK.
The caller may choose to pass both some
The domain destroy would not change cfg at all.
Okay.
If not, I should double check this duplication so what about a return in
the case of duplicating one entry?
What I am looking for is a *decision* about what the right behaviour
is, backed up by a *rationale*.
The most obvious answer
On 2015/7/21 19:11, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
I would add this check at the beginning of this function:
assert(!d_config-num_rdms !d_config-rdms).
As Ian Campbell and I have explained, that is not OK.
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
On 2015/7/21 19:11, Ian Jackson wrote:
The most obvious case is something like the following code
libxl_domain_config cfg;
cfg.stuff = blah;
cfg.rdm.strategy = HOST;
On 2015/7/21 19:11, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
I would add this check at the beginning of this function:
assert(!d_config-num_rdms !d_config-rdms).
As Ian Campbell and I have explained, that is not OK.
But d_config is a libxl_domain_config which is supplied by libxl's
caller. It might contain some rdms.
I guess this line make you or other guys confused so lets delete this
line directly.
I don't think I am very confused.
And if you still worry about something, I can add assert() at the
I think the confusion here is that the d_config-rdms array (which
num_rdms is the length of) is in the public API (because it is in
libxl_types.idl) but is apparently only being used in this series as an
internal state for the domain build process (i.e. xl doesn't ever add
anything to the
On 2015/7/21 20:33, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
[Ian Jackson:]
The most obvious answer to me would be that if an rdms array is
specified, the strategy should be ignored. That is how the automatic
numa
But another answer would be to take the union of the specified
regions. That would be more complicated, because the union would have
to be computed.
if (d_config-rdms[i].start == rdm_start)
return;
That doesn't, of course, compute the union.
Sorry I don't
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
If you're talking to create the domain twice I think this should be
reasonable,
Right.
if (d_config-num_rdms) return 0;
Yes.
Because RDMs are always associated to the platform so we should keep
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
[Ian Jackson:]
The most obvious answer to me would be that if an rdms array is
specified, the strategy should be ignored. That is how the automatic
numa placement API works.
I'm not familiar
While building a VM, HVM domain builder provides struct hvm_info_table{}
to help hvmloader. Currently it includes two fields to construct guest
e820 table by hvmloader, low_mem_pgend and high_mem_pgend. So we should
check them to fix any conflict with RDM.
RMRR can reside in address space beyond
Note I need more time to address others.
+int libxl__domain_device_construct_rdm(libxl__gc *gc,
+ libxl_domain_config *d_config,
+ uint64_t rdm_mem_boundary,
+ struct
Tiejun Chen writes ([v10][PATCH 11/16] tools/libxl: detect and avoid conflicts
with RDM):
While building a VM, HVM domain builder provides struct hvm_info_table{}
to help hvmloader. Currently it includes two fields to construct guest
e820 table by hvmloader, low_mem_pgend and high_mem_pgend.
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and avoid
conflicts with RDM):
Note I need more time to address others.
Right. But thanks for coming back quickly with this question:
+int libxl__domain_device_construct_rdm(libxl__gc *gc,
+
+int libxl__domain_device_construct_rdm(libxl__gc *gc,
+ libxl_domain_config *d_config,
+ uint64_t rdm_mem_boundary,
+ struct xc_hvm_build_args *args)
+{
...
+/* Query all RDM
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and
avoid conflicts with RDM):
[Ian Jackson:]
The domain configuration specified to libxl might contain some rdms.
Then num_rdms in the incoming config would be nonzero.
We never set d_config-num_rdms/d_config-rdms before we
On Mon, 2015-07-20 at 16:24 +0100, Ian Jackson wrote:
Chen, Tiejun writes (Re: [v10][PATCH 11/16] tools/libxl: detect and
avoid conflicts with RDM):
[Ian Jackson:]
The domain configuration specified to libxl might contain some rdms.
Then num_rdms in the incoming config would be nonzero.
31 matches
Mail list logo
