| Message ID | 20170810112428.5769-2-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/17 12:24, Juergen Gross wrote: > Today the maximum record lenth in a migration stream is 8MB. This > limits the size of a PV domain to a little bit less than 1TB in the > migration case, as the P2M frame list will exceed 8MB in this case. > > Raising the record size limit by a factor of 16 allows for domain > sizes of nearly 16TB to be migrated. This ought to be enough. > > Signed-off-by: Juergen Gross <jgross@suse.com> Hmm - Changing this isn't something I've considered when it comes to ABI compatibility. I also see that there is no mention of the maximum record length in the stream spec, which is an oversight. Worse still, there is no record length check in the python utilities, but both sides of the C code perform the check. Let me ponder the implications. ~Andrew
On 10/08/17 14:26, Andrew Cooper wrote: > On 10/08/17 12:24, Juergen Gross wrote: >> Today the maximum record lenth in a migration stream is 8MB. This >> limits the size of a PV domain to a little bit less than 1TB in the >> migration case, as the P2M frame list will exceed 8MB in this case. >> >> Raising the record size limit by a factor of 16 allows for domain >> sizes of nearly 16TB to be migrated. This ought to be enough. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> > > Hmm - Changing this isn't something I've considered when it comes to ABI > compatibility. I also see that there is no mention of the maximum > record length in the stream spec, which is an oversight. > > Worse still, there is no record length check in the python utilities, > but both sides of the C code perform the check. > > Let me ponder the implications. Any result yet? Juergen
Andrew Cooper writes ("Re: [PATCH] libxc: increase maximum migration stream record length"):
> On 10/08/17 12:24, Juergen Gross wrote:
> > Today the maximum record lenth in a migration stream is 8MB. This
> > limits the size of a PV domain to a little bit less than 1TB in the
> > migration case, as the P2M frame list will exceed 8MB in this case.
> >
> > Raising the record size limit by a factor of 16 allows for domain
> > sizes of nearly 16TB to be migrated. This ought to be enough.
> >
> > Signed-off-by: Juergen Gross <jgross@suse.com>
>
> Hmm - Changing this isn't something I've considered when it comes to ABI
> compatibility. I also see that there is no mention of the maximum
> record length in the stream spec, which is an oversight.
>
> Worse still, there is no record length check in the python utilities,
> but both sides of the C code perform the check.
>
> Let me ponder the implications.
I think simply changing this #define is the right approach.
What we mostly care about is that old senders can successfully send
data to new receivers, for which this is not an issue.
As regards new senders and old receivers:
This #define is not used to actually control the size of outgoing
records. The only mentions are in safety checks, in both sending and
receiving side.
Therefore, in a situation where the old code would generate a
particular stream without error, the new code would generate exactly
the same stream. (Likewise, obviously, the interpretation of existing
valid streams is not changed.)
The only difference in behaviour is that in some situations the old
sender will throw an error and abandon the migration attempt. In
these same situations the new sender will generate a stream which will
be rejected by old receivers, but accepted by new receivers.
So increasing this #define is good:
* All existing workin use cases work as before.
* The new use case works with new code at both ends
(this is the best that can be done because AIUI there is
no way to represent this domain in a way that the old code
would understand - although I have not verified this).
* In one of the non-working cases the error handling is changed:
the error is now detected at the receiver rather than at the
sender. This is, however, fine.
So:
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian.
On Thu, Aug 31, 2017 at 11:24:23AM +0100, Ian Jackson wrote: > Andrew Cooper writes ("Re: [PATCH] libxc: increase maximum migration stream record length"): > > On 10/08/17 12:24, Juergen Gross wrote: > > > Today the maximum record lenth in a migration stream is 8MB. This > > > limits the size of a PV domain to a little bit less than 1TB in the > > > migration case, as the P2M frame list will exceed 8MB in this case. > > > > > > Raising the record size limit by a factor of 16 allows for domain > > > sizes of nearly 16TB to be migrated. This ought to be enough. > > > > > > Signed-off-by: Juergen Gross <jgross@suse.com> > > > > Hmm - Changing this isn't something I've considered when it comes to ABI > > compatibility. I also see that there is no mention of the maximum > > record length in the stream spec, which is an oversight. > > > > Worse still, there is no record length check in the python utilities, > > but both sides of the C code perform the check. > > > > Let me ponder the implications. > > I think simply changing this #define is the right approach. > > What we mostly care about is that old senders can successfully send > data to new receivers, for which this is not an issue. > > As regards new senders and old receivers: > > This #define is not used to actually control the size of outgoing > records. The only mentions are in safety checks, in both sending and > receiving side. > > Therefore, in a situation where the old code would generate a > particular stream without error, the new code would generate exactly > the same stream. (Likewise, obviously, the interpretation of existing > valid streams is not changed.) > > The only difference in behaviour is that in some situations the old > sender will throw an error and abandon the migration attempt. In > these same situations the new sender will generate a stream which will > be rejected by old receivers, but accepted by new receivers. > > So increasing this #define is good: > > * All existing workin use cases work as before. > > * The new use case works with new code at both ends > (this is the best that can be done because AIUI there is > no way to represent this domain in a way that the old code > would understand - although I have not verified this). > > * In one of the non-working cases the error handling is changed: > the error is now detected at the receiver rather than at the > sender. This is, however, fine. > > So: > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Applied.
diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h index 3291b256fd..15ff1c7efb 100644 --- a/tools/libxc/xc_sr_stream_format.h +++ b/tools/libxc/xc_sr_stream_format.h @@ -57,8 +57,8 @@ struct xc_sr_rhdr /* All records must be aligned up to an 8 octet boundary */ #define REC_ALIGN_ORDER (3U) -/* Somewhat arbitrary - 8MB */ -#define REC_LENGTH_MAX (8U << 20) +/* Somewhat arbitrary - 128MB */ +#define REC_LENGTH_MAX (128U << 20) #define REC_TYPE_END 0x00000000U #define REC_TYPE_PAGE_DATA 0x00000001U
Today the maximum record lenth in a migration stream is 8MB. This limits the size of a PV domain to a little bit less than 1TB in the migration case, as the P2M frame list will exceed 8MB in this case. Raising the record size limit by a factor of 16 allows for domain sizes of nearly 16TB to be migrated. This ought to be enough. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/libxc/xc_sr_stream_format.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)