On Wed, Apr 15, 2015 at 04:25:11PM +0530, Prerna Saxena wrote:
On Monday 13 April 2015 07:50 PM, Daniel P. Berrange wrote:On Mon, Apr 13, 2015 at 04:14:53PM +0200, Martin Kletzander wrote:Use the same pattern as there is for x86 machines. Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- docs/schemas/domaincommon.rng | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..80b30df 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -384,7 +384,9 @@ </optional> <optional> <attribute name="machine"> - <value>sun4m</value> + <data type="string"> + <param name="pattern">[a-zA-Z0-9_\.\-]+</param> + </data> </attribute> </optional> </group>I think you could probably simplify this all much more. All these architecture specific blocks of machine type names should just be deleted and so this: <define name="ostypehvm"> <element name="type"> <optional> <choice> <ref name="hvmx86"/> <ref name="hvmmips"/> <ref name="hvmsparc"/> <ref name="hvmppc"/> <ref name="hvmppc64"/> <ref name="hvms390"/> <ref name="hvmarm"/> <ref name="hvmaarch64"/> </choice> </optional> <value>hvm</value> </element> </define> Would simplify to just <define name="ostypehvm"> <element name="type"> <optional> <attribute name="arch"> <choice> <value>i686</value> ....others... </choice> </attribute> </optional> <optional> <attribute name="machine"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\.\-]+</param> </data> </attribute> </optional> </element> </define>Hi Martin, Daniel, I have not been able to try this patch, it fails with this error : error: internal error: Unable to parse RNG /test-libvirt/share/libvirt/schemas/domain.rng: Reference osexe has no matching definition Internal found no define for ref osexe However, had some concerns purely by looking at this patch. This change is very x86-centric, it does not respect other architectures. I think the rationale for simplifying domaincommon.rng would have been to group all types that obey this pattern string: <param name="pattern">[a-zA-Z0-9_\.\-]+</param> However, this regex does not conform to machine types for _all_ architectures. As an example, see this : <define name="hvms390"> <group> <optional> <attribute name="arch"> <choice> <value>s390</value> <value>s390x</value> </choice> </attribute> </optional> <optional> <attribute name="machine"> <choice> <value>s390</value> <value>s390-virtio</value> <value>s390-ccw</value> <value>s390-ccw-virtio</value> </choice> </attribute> </optional> </group> </define> The s390 arch only allows four machine names : "s390", "s390-virtio", "s390-ccw", "s390-ccw-virtio". With the patch you suggest, even a string such as "abcdefg" will become a legitimate machine type for s390x, which seems like an odd thing. Likewise, ppc64[le] architecture allows only strings such as pseries, pseries-2.1, pseries-2.2 .. This patch will allow any random machine name, which seems somewhat odd to me.
Well, that's now. If new QEMU comes with new machine type, we'll have to fix libvirt to work with that. I was trying to look into the future a bit. Anyway, there'll be a v3, so that's going to be the place to make discussions.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list