From: ray <honglei.wang@smartx.com>

Signed-off-by: ray <honglei.wang@smartx.com>
---
 src/conf/schemas/domaincommon.rng | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 5597d5a66b..7bdad3c793 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2518,7 +2518,7 @@
 
   <define name="diskTargetDev">
     <data type="string">
-      <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param>
+      <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd|nvmens)[a-zA-Z0-9_]+</param>
     </data>
   </define>
 
@@ -2539,6 +2539,7 @@
             <value>uml</value> <!-- NOT USED ANYMORE -->
             <value>sata</value>
             <value>sd</value>
+            <value>nvme-ns</value>
           </choice>
         </attribute>
       </optional>
@@ -3044,6 +3045,14 @@
               </attribute>
             </optional>
           </group>
+          <group>
+            <attribute name="type">
+              <value>nvme</value>
+            </attribute>
+            <element name="serial">
+              <ref name="diskSerial"/>
+            </element>
+          </group>
         </choice>
         <optional>
           <element name="driver">
-- 
2.11.0

On Sun, Apr 27, 2025 at 19:48:05 +0800, honglei.wang@smartx.com wrote:
> From: ray <honglei.wang@smartx.com>
> 
> Signed-off-by: ray <honglei.wang@smartx.com>
> ---
>  src/conf/schemas/domaincommon.rng | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 5597d5a66b..7bdad3c793 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -2518,7 +2518,7 @@
>  
>    <define name="diskTargetDev">
>      <data type="string">
> -      <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param>
> +      <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd|nvmens)[a-zA-Z0-9_]+</param>
>      </data>
>    </define>
>  
> @@ -2539,6 +2539,7 @@
>              <value>uml</value> <!-- NOT USED ANYMORE -->
>              <value>sata</value>
>              <value>sd</value>
> +            <value>nvme-ns</value>
>            </choice>
>          </attribute>
>        </optional>
> @@ -3044,6 +3045,14 @@
>                </attribute>
>              </optional>
>            </group>
> +          <group>
> +            <attribute name="type">
> +              <value>nvme</value>
> +            </attribute>
> +            <element name="serial">
> +              <ref name="diskSerial"/>
> +            </element>

So here the 'serial' is declared as mandatory. It's optional in the XML
parser/formatter and mandatory in the commandline formatter. With other
devices it's optional so it's most likely going to need an <optional>
block.

This series is also completely lacking documentation
(docs/formatdomain.rst) documenting bot the new controller and disk
type.

> 
> So here the 'serial' is declared as mandatory. It's optional in the XML
> parser/formatter and mandatory in the commandline formatter. With other
> devices it's optional so it's most likely going to need an <optional>
> block.
> 
> This series is also completely lacking documentation
> (docs/formatdomain.rst) documenting bot the new controller and disk
> type.
As described earlier, the intention is for the serial field to be mandatory. Perhaps the parser/formatter handling led to some confusion? Any suggestions are very welcome.
I'll add documentation for it in docs/formatdomain.rst.

On Fri, May 09, 2025 at 08:38:43 -0000, ray wang wrote:
> > 
> > So here the 'serial' is declared as mandatory. It's optional in the XML
> > parser/formatter and mandatory in the commandline formatter. With other
> > devices it's optional so it's most likely going to need an <optional>
> > block.
> > 
> > This series is also completely lacking documentation
> > (docs/formatdomain.rst) documenting bot the new controller and disk
> > type.
> As described earlier, the intention is for the serial field to be mandatory. Perhaps the parser/formatter handling led to some confusion? Any suggestions are very welcome.
> I'll add documentation for it in docs/formatdomain.rst.

It was unclear due to the discrepancies in the code and the lacking
documentation couldn't explain it.

It's okay if you need to make it mandatory, in which case add a
validation check (src/qemu/qemu_validate.c) reporting a user-friendly
error.