Skip to content
  • Markus Armbruster's avatar
    e5af0da1
    block: Fix -blockdev for certain non-string scalars · e5af0da1
    Markus Armbruster authored
    
    
    Configuration flows through the block subsystem in a rather peculiar
    way.  Configuration made with -drive enters it as QemuOpts.
    Configuration made with -blockdev / blockdev-add enters it as QAPI
    type BlockdevOptions.  The block subsystem uses QDict, QemuOpts and
    QAPI types internally.  The precise flow is next to impossible to
    explain (I tried for this commit message, but gave up after wasting
    several hours).  What I can explain is a flaw in the BlockDriver
    interface that leads to this bug:
    
        $ qemu-system-x86_64 -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234
        qemu-system-x86_64: -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234: Internal error: parameter user invalid
    
    QMP blockdev-add is broken the same way.
    
    Here's what happens.  The block layer passes configuration represented
    as flat QDict (with dotted keys) to BlockDriver methods
    .bdrv_file_open().  The QDict's members are typed according to the
    QAPI schema.
    
    nfs_file_open() converts it to QAPI type BlockdevOptionsNfs, with
    qdict_crumple() and a qobject input visitor.
    
    This visitor comes in two flavors.  The plain flavor requires scalars
    to be typed according to the QAPI schema.  That's the case here.  The
    keyval flavor requires string scalars.  That's not the case here.
    nfs_file_open() uses the latter, and promptly falls apart for members
    @user, @group, @tcp-syn-count, @readahead-size, @page-cache-size,
    @debug.
    
    Switching to the plain flavor would fix -blockdev, but break -drive,
    because there the scalars arrive in nfs_file_open() as strings.
    
    The proper fix would be to replace the QDict by QAPI type
    BlockdevOptions in the BlockDriver interface.  Sadly, that's beyond my
    reach right now.
    
    Next best would be to fix the block layer to always pass correctly
    typed QDicts to the BlockDriver methods.  Also beyond my reach.
    
    What I can do is throw another hack onto the pile: have
    nfs_file_open() convert all members to string, so use of the keyval
    flavor actually works, by replacing qdict_crumple() by new function
    qdict_crumple_for_keyval_qiv().
    
    The pattern "pass result of qdict_crumple() to
    qobject_input_visitor_new_keyval()" occurs several times more:
    
    * qemu_rbd_open()
    
      Same issue as nfs_file_open(), but since BlockdevOptionsRbd has only
      string members, its only a latent bug.  Fix it anyway.
    
    * parallels_co_create_opts(), qcow_co_create_opts(),
      qcow2_co_create_opts(), bdrv_qed_co_create_opts(),
      sd_co_create_opts(), vhdx_co_create_opts(), vpc_co_create_opts()
    
      These work, because they create the QDict with
      qemu_opts_to_qdict_filtered(), which creates only string scalars.
      The function sports a TODO comment asking for better typing; that's
      going to be fun.  Use qdict_crumple_for_keyval_qiv() to be safe.
    
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    e5af0da1
    block: Fix -blockdev for certain non-string scalars
    Markus Armbruster authored
    
    
    Configuration flows through the block subsystem in a rather peculiar
    way.  Configuration made with -drive enters it as QemuOpts.
    Configuration made with -blockdev / blockdev-add enters it as QAPI
    type BlockdevOptions.  The block subsystem uses QDict, QemuOpts and
    QAPI types internally.  The precise flow is next to impossible to
    explain (I tried for this commit message, but gave up after wasting
    several hours).  What I can explain is a flaw in the BlockDriver
    interface that leads to this bug:
    
        $ qemu-system-x86_64 -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234
        qemu-system-x86_64: -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234: Internal error: parameter user invalid
    
    QMP blockdev-add is broken the same way.
    
    Here's what happens.  The block layer passes configuration represented
    as flat QDict (with dotted keys) to BlockDriver methods
    .bdrv_file_open().  The QDict's members are typed according to the
    QAPI schema.
    
    nfs_file_open() converts it to QAPI type BlockdevOptionsNfs, with
    qdict_crumple() and a qobject input visitor.
    
    This visitor comes in two flavors.  The plain flavor requires scalars
    to be typed according to the QAPI schema.  That's the case here.  The
    keyval flavor requires string scalars.  That's not the case here.
    nfs_file_open() uses the latter, and promptly falls apart for members
    @user, @group, @tcp-syn-count, @readahead-size, @page-cache-size,
    @debug.
    
    Switching to the plain flavor would fix -blockdev, but break -drive,
    because there the scalars arrive in nfs_file_open() as strings.
    
    The proper fix would be to replace the QDict by QAPI type
    BlockdevOptions in the BlockDriver interface.  Sadly, that's beyond my
    reach right now.
    
    Next best would be to fix the block layer to always pass correctly
    typed QDicts to the BlockDriver methods.  Also beyond my reach.
    
    What I can do is throw another hack onto the pile: have
    nfs_file_open() convert all members to string, so use of the keyval
    flavor actually works, by replacing qdict_crumple() by new function
    qdict_crumple_for_keyval_qiv().
    
    The pattern "pass result of qdict_crumple() to
    qobject_input_visitor_new_keyval()" occurs several times more:
    
    * qemu_rbd_open()
    
      Same issue as nfs_file_open(), but since BlockdevOptionsRbd has only
      string members, its only a latent bug.  Fix it anyway.
    
    * parallels_co_create_opts(), qcow_co_create_opts(),
      qcow2_co_create_opts(), bdrv_qed_co_create_opts(),
      sd_co_create_opts(), vhdx_co_create_opts(), vpc_co_create_opts()
    
      These work, because they create the QDict with
      qemu_opts_to_qdict_filtered(), which creates only string scalars.
      The function sports a TODO comment asking for better typing; that's
      going to be fun.  Use qdict_crumple_for_keyval_qiv() to be safe.
    
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Loading