[PATCH v4 00/13] tools/xenstore: rework internal accounting

Juergen Gross posted 13 patches 1 year ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230405070349.25293-1-jgross@suse.com
There is a newer version of this series
docs/misc/xenstore.txt                 |   5 +-
tools/xenstore/xenstored_control.c     |  65 ++--
tools/xenstore/xenstored_core.c        | 168 +++++-----
tools/xenstore/xenstored_core.h        |  24 +-
tools/xenstore/xenstored_domain.c      | 433 ++++++++++++++++++-------
tools/xenstore/xenstored_domain.h      |  60 +++-
tools/xenstore/xenstored_transaction.c |  22 +-
tools/xenstore/xenstored_watch.c       |  15 +-
8 files changed, 514 insertions(+), 278 deletions(-)
[PATCH v4 00/13] tools/xenstore: rework internal accounting
Posted by Juergen Gross 1 year ago
This series reworks the Xenstore internal accounting to use a uniform
generic framework. It is adding some additional useful diagnostic
information, like accounting trace and max. per-domain and global quota
values seen.

Changes in V2:
- added patch 1 (leftover from previous series)
- rebase

Changes in V3:
- addressed comments

Changes in V4:
- fixed patch 3

Juergen Gross (13):
  tools/xenstore: take transaction internal nodes into account for quota
  tools/xenstore: manage per-transaction domain accounting data in an
    array
  tools/xenstore: introduce accounting data array for per-domain values
  tools/xenstore: add framework to commit accounting data on success
    only
  tools/xenstore: use accounting buffering for node accounting
  tools/xenstore: add current connection to domain_memory_add()
    parameters
  tools/xenstore: use accounting data array for per-domain values
  tools/xenstore: add accounting trace support
  tools/xenstore: add TDB access trace support
  tools/xenstore: switch transaction accounting to generic accounting
  tools/xenstore: remember global and per domain max accounting values
  tools/xenstore: use generic accounting for remaining quotas
  tools/xenstore: switch quota management to be table based

 docs/misc/xenstore.txt                 |   5 +-
 tools/xenstore/xenstored_control.c     |  65 ++--
 tools/xenstore/xenstored_core.c        | 168 +++++-----
 tools/xenstore/xenstored_core.h        |  24 +-
 tools/xenstore/xenstored_domain.c      | 433 ++++++++++++++++++-------
 tools/xenstore/xenstored_domain.h      |  60 +++-
 tools/xenstore/xenstored_transaction.c |  22 +-
 tools/xenstore/xenstored_watch.c       |  15 +-
 8 files changed, 514 insertions(+), 278 deletions(-)

-- 
2.35.3
Re: [PATCH v4 00/13] tools/xenstore: rework internal accounting
Posted by Juergen Gross 1 year ago
On 05.04.23 09:03, Juergen Gross wrote:
> This series reworks the Xenstore internal accounting to use a uniform
> generic framework. It is adding some additional useful diagnostic
> information, like accounting trace and max. per-domain and global quota
> values seen.
> 
> Changes in V2:
> - added patch 1 (leftover from previous series)
> - rebase
> 
> Changes in V3:
> - addressed comments
> 
> Changes in V4:
> - fixed patch 3

Another thought for this series and followup one:

Do we want to keep current coding style in tools/xenstore (basically
Linux kernel style), or do we want to switch to Xen style instead?

If a switch to Xen style is preferred (I do prefer that switch), I'd
like to suggest that I do a rework of this series and the followup one
to use the Xen style for new or moved functions.

A more radical approach would be to do a large style switch series
after the two series, but I'm hesitant as this would affect backports
rather badly.


Juergen

Re: [PATCH v4 00/13] tools/xenstore: rework internal accounting
Posted by Julien Grall 1 year ago
Hi Juergen,

On 26/04/2023 08:19, Juergen Gross wrote:
> On 05.04.23 09:03, Juergen Gross wrote:
>> This series reworks the Xenstore internal accounting to use a uniform
>> generic framework. It is adding some additional useful diagnostic
>> information, like accounting trace and max. per-domain and global quota
>> values seen.
>>
>> Changes in V2:
>> - added patch 1 (leftover from previous series)
>> - rebase
>>
>> Changes in V3:
>> - addressed comments
>>
>> Changes in V4:
>> - fixed patch 3
> 
> Another thought for this series and followup one:
> 
> Do we want to keep current coding style in tools/xenstore (basically
> Linux kernel style), or do we want to switch to Xen style instead?

I am a bit split on this one. I don't particularly like the Linux coding 
style, but it has the advantage to be well-documented (if we compare to 
the Xen one).

May I ask what would be the reason to switch?

> 
> If a switch to Xen style is preferred (I do prefer that switch), I'd
> like to suggest that I do a rework of this series and the followup one
> to use the Xen style for new or moved functions.

I think this is a bad idea because it would make difficult for a 
developer/reviewer to know what is the coding style of a given function.

At least in my workflow, it would also means that I need two open the 
file twice with different settings (e.g. soft vs hard tab).

> 
> A more radical approach would be to do a large style switch series
> after the two series, but I'm hesitant as this would affect backports
> rather badly.

In general, I would agree with that. But, after your work, I am under 
the impression that Xenstored will become quite different. So I am not 
convince we will be able to backports a lot of patch without significant 
rework.

Therefore, converting all the files in one pass may not be too bad 
(assuming we agree on switching to the new coding style).

Cheers,

-- 
Julien Grall
Re: [PATCH v4 00/13] tools/xenstore: rework internal accounting
Posted by Juergen Gross 1 year ago
On 27.04.23 19:37, Julien Grall wrote:
> Hi Juergen,
> 
> On 26/04/2023 08:19, Juergen Gross wrote:
>> On 05.04.23 09:03, Juergen Gross wrote:
>>> This series reworks the Xenstore internal accounting to use a uniform
>>> generic framework. It is adding some additional useful diagnostic
>>> information, like accounting trace and max. per-domain and global quota
>>> values seen.
>>>
>>> Changes in V2:
>>> - added patch 1 (leftover from previous series)
>>> - rebase
>>>
>>> Changes in V3:
>>> - addressed comments
>>>
>>> Changes in V4:
>>> - fixed patch 3
>>
>> Another thought for this series and followup one:
>>
>> Do we want to keep current coding style in tools/xenstore (basically
>> Linux kernel style), or do we want to switch to Xen style instead?
> 
> I am a bit split on this one. I don't particularly like the Linux coding style, 
> but it has the advantage to be well-documented (if we compare to the Xen one).

I have raised the idea to switch to the Linux style for that reason, but it was
rejected rather firmly.

So we won't get rid of the Xen style.

> May I ask what would be the reason to switch?

According to CODING_STYLE it is the style to be used. We could add a local style
hint in tools/xenstored, but I'd rather not add another local style.

In the end it is about consistency.

>> If a switch to Xen style is preferred (I do prefer that switch), I'd
>> like to suggest that I do a rework of this series and the followup one
>> to use the Xen style for new or moved functions.
> 
> I think this is a bad idea because it would make difficult for a 
> developer/reviewer to know what is the coding style of a given function.
> 
> At least in my workflow, it would also means that I need two open the file twice 
> with different settings (e.g. soft vs hard tab).

Okay. This is a rather good reason not to use different styles in one source.

>> A more radical approach would be to do a large style switch series
>> after the two series, but I'm hesitant as this would affect backports
>> rather badly.
> 
> In general, I would agree with that. But, after your work, I am under the 
> impression that Xenstored will become quite different. So I am not convince we 
> will be able to backports a lot of patch without significant rework.
> 
> Therefore, converting all the files in one pass may not be too bad (assuming we 
> agree on switching to the new coding style).

Fine with me. In any case this should be done in the same Xen release as the
major rework. Otherwise the backport problem will be a real one.


Juergen