On Thu, Jun 06, 2024 at 08:07:37 -0000, Chun Feng Wu wrote: > for comment > "Preferrably all these definitions should be shared with <iotune> as any > change would now need to change two places." Trimming the context and mentioning this random bit makes it rather hard to remember what I've based that comment on ... > > do you mean we use domain xml like the following one(which reuse <iotune> definition): ... so I have no idea how to respond to this without going back myself. Plese preferably keep the context you are responding to rather than starting a random new message. Let me do that for you now: > > @@ -6637,6 +6641,164 @@ > > </interleave> > > </element> > > </define> > > + <!-- > > + throttlegroup is similar to <iotune>, however, group_name is required, which is different from <iotune> > > + --> > > + <define name="throttlegroup"> > > + <element name="throttlegroup"> > > + <interleave> > > + <!-- required --> > > + <element name="group_name"> > > + <text/> > > + </element> > > + <choice> > > + <element name="total_bytes_sec"> > > + <data type="unsignedLong"/> > > + </element> > > Preferrably all these definitions should be shared with <iotune> as any > change would now need to change two places. > > It is okay if the schema is more lax (e.g. allowing 'group_name' to be > optional in the schema, which is then strictly required by the parser), > but it's not okay to have to change two distinct places. > > > + <group> > > + <interleave> > > + <optional> > > + <element name="read_bytes_sec"> > > + <data type="unsignedLong"/> > > + </element> > > do you mean we use domain xml like the following one(which reuse <iotune> definition): > <domain> > ... > <iotunes> > <iotune> > <group_name>limit0</group_name> > <total_bytes_sec>10000000</total_bytes_sec> > <read_iops_sec>400000</read_iops_sec> > <write_iops_sec>100000</write_iops_sec> > </iotune> > </iotunes> > ... > </devices> > <disk type='file' device='disk'> > <driver name='qemu' type='qcow2' /> > <source file='/var/lib/libvirt/images/disk.qcow2'/> > <target dev='vdh' bus='virtio'/> > <throttlefilters> > <throttlefilter iotune='limit2'/> > <throttlefilter iotune='limit012'/> No, what I meant is to not reimplement the whole definition on the schema jus tto make 'group_name' required by the schema itself. You can make it required in the parser, and keep it optional in the schema so that the schema can be reused in both places rather than having two distinct copies of it that differ just in the optionality of 'group_name'.
