| Message ID | 1425971164-9845-1-git-send-email-mjt@msgid.tls.msk.ru |
|---|---|
| State | New |
| Headers | show |
Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > block/dmg can use additional library (libbz2) to read > bzip2-compressed files. Make the block driver to be > a module if libbz2 support is requested, to avoid extra > library dependency by default. > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> First of all: I don't think this is suitable for trivial. The actual code change might be small, but the change in behaviour is important and needs discussion. > This might be questionable, to make the thing to be either > module or built-in depending on build environment, so a > better idea may be to make it modular unconditionally. > This block device format isn't used often. Yes, I'm concerned that making it conditional might be a bit surprising. I'd like to hear some more opinions before applying this. Also, should we consider making some more rarely used image formats modules even if they don't pull in external dependencies? Kevin
On Tue, Mar 10, 2015 at 10:06:04AM +0300, Michael Tokarev wrote: > block/dmg can use additional library (libbz2) to read > bzip2-compressed files. Make the block driver to be > a module if libbz2 support is requested, to avoid extra > library dependency by default. > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Tested-by: Peter Wu <peter@lekensteyn.nl> Reviewed-by: Peter Wu <peter@lekensteyn.nl> > -- > > This might be questionable, to make the thing to be either > module or built-in depending on build environment, so a > better idea may be to make it modular unconditionally. > This block device format isn't used often. I do not mind making it a module unconditionally, that would make it easier to disable should a security bug come around the corner. Kind regards, Peter > block/Makefile.objs | 3 ++- > configure | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/block/Makefile.objs b/block/Makefile.objs > index db2933e..440c51f 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -1,7 +1,8 @@ > -block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o > +block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o > block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o > block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o > block-obj-y += qed-check.o > +block-obj-$(CONFIG_DMG) += dmg.o > block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o > block-obj-$(CONFIG_QUORUM) += quorum.o > block-obj-y += parallels.o blkdebug.o blkverify.o > diff --git a/configure b/configure > index 7ba4bcb..1dd5721 100755 > --- a/configure > +++ b/configure > @@ -4772,6 +4772,9 @@ fi > if test "$bzip2" = "yes" ; then > echo "CONFIG_BZIP2=y" >> $config_host_mak > echo "BZIP2_LIBS=-lbz2" >> $config_host_mak > + echo "CONFIG_DMG=m" >> $config_host_mak > +else > + echo "CONFIG_DMG=y" >> $config_host_mak > fi > > if test "$libiscsi" = "yes" ; then > -- > 2.1.4 >
On Tue, 03/10 09:50, Kevin Wolf wrote: > Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > > block/dmg can use additional library (libbz2) to read > > bzip2-compressed files. Make the block driver to be > > a module if libbz2 support is requested, to avoid extra > > library dependency by default. > > > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > > First of all: I don't think this is suitable for trivial. The actual > code change might be small, but the change in behaviour is important and > needs discussion. > > > This might be questionable, to make the thing to be either > > module or built-in depending on build environment, so a > > better idea may be to make it modular unconditionally. > > This block device format isn't used often. > > Yes, I'm concerned that making it conditional might be a bit surprising. > I'd like to hear some more opinions before applying this. I don't see the advantage over making it an unconditional module - condition only makes it a bit more complicated. > > Also, should we consider making some more rarely used image formats > modules even if they don't pull in external dependencies? Sounds reasonable to me. Is the intention to reduce binary size? Fam
Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: > On Tue, 03/10 09:50, Kevin Wolf wrote: > > Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > > > block/dmg can use additional library (libbz2) to read > > > bzip2-compressed files. Make the block driver to be > > > a module if libbz2 support is requested, to avoid extra > > > library dependency by default. > > > > > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > > > > First of all: I don't think this is suitable for trivial. The actual > > code change might be small, but the change in behaviour is important and > > needs discussion. > > > > > This might be questionable, to make the thing to be either > > > module or built-in depending on build environment, so a > > > better idea may be to make it modular unconditionally. > > > This block device format isn't used often. > > > > Yes, I'm concerned that making it conditional might be a bit surprising. > > I'd like to hear some more opinions before applying this. > > I don't see the advantage over making it an unconditional module - condition > only makes it a bit more complicated. > > > > > Also, should we consider making some more rarely used image formats > > modules even if they don't pull in external dependencies? > > Sounds reasonable to me. Is the intention to reduce binary size? Yes, that and also that it allows compiling out some drivers without having to mess with the Makefiles. You just don't install all of them. Related to that, Peter also mentioned that you (the user, not developer or packager) could simply disable a single driver, for example as a temporary hotfix in the case of security problems in a block driver. That would actually be an argument for making _all_ drivers modules. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: >> On Tue, 03/10 09:50, Kevin Wolf wrote: >> > Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: >> > > block/dmg can use additional library (libbz2) to read >> > > bzip2-compressed files. Make the block driver to be >> > > a module if libbz2 support is requested, to avoid extra >> > > library dependency by default. >> > > >> > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> >> > >> > First of all: I don't think this is suitable for trivial. The actual >> > code change might be small, but the change in behaviour is important and >> > needs discussion. >> > >> > > This might be questionable, to make the thing to be either >> > > module or built-in depending on build environment, so a >> > > better idea may be to make it modular unconditionally. >> > > This block device format isn't used often. >> > >> > Yes, I'm concerned that making it conditional might be a bit surprising. >> > I'd like to hear some more opinions before applying this. >> >> I don't see the advantage over making it an unconditional module - condition >> only makes it a bit more complicated. >> >> > >> > Also, should we consider making some more rarely used image formats >> > modules even if they don't pull in external dependencies? >> >> Sounds reasonable to me. Is the intention to reduce binary size? Fair argument, but not a paricularly weighty one. The unused code never gets paged in, mostly, and isn't particularly large to begin with. > Yes, that and also that it allows compiling out some drivers without > having to mess with the Makefiles. You just don't install all of them. Second best solution of the configuration management problem "select a subset of the available block drivers", good enough when compiling the unused ones isn't bothersome. But the best solution is still configuration management capable of disabling optional components. > Related to that, Peter also mentioned that you (the user, not developer > or packager) could simply disable a single driver, for example as a > temporary hotfix in the case of security problems in a block driver. > That would actually be an argument for making _all_ drivers modules. And pretty much every other optional component. For me, avoiding bothersome dependencies is a strong practical argument for making a something a loadable module. Other benefits of loadable modules presented so far seem pretty negligible to me. If you want them, no objection from me, as long as the cost is similarly negligible, additional complexity for developers, packagers and users in particular.
10.03.2015 16:24, Markus Armbruster wrote: [] > For me, avoiding bothersome dependencies is a strong practical argument > for making a something a loadable module. Other benefits of loadable > modules presented so far seem pretty negligible to me. If you want > them, no objection from me, as long as the cost is similarly negligible, > additional complexity for developers, packagers and users in particular. That's the same for me. No need to reduce code size, at least not by that much, and no need to disable some "insecure" eg block driver, but once something pulls in some interesting external dep it seems to be worth the effort (if it is not large anyway) to make it loadable. It is much bigger PITA if some feature is needed in some obscure or rare sutuation but it is not present -- cost of solving this might be significantly larger than billions of copies of unused code on billions of user's hdds :) This is the reason I made it a module or built-in depending on usage of external dep. However, in this case libbz2 is most likely present on a user's system anyway (since at least some time ago it was rather popular, not as wide as gzip but still significantly; now it is mostly replaced by xz), so there isn't much of an external dependency. Thanks, /mjt
On Tue, Mar 10, 2015 at 10:09 AM, Kevin Wolf <kwolf@redhat.com> wrote: > Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: >> On Tue, 03/10 09:50, Kevin Wolf wrote: >> > Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: >> > Also, should we consider making some more rarely used image formats >> > modules even if they don't pull in external dependencies? >> >> Sounds reasonable to me. Is the intention to reduce binary size? > > Yes, that and also that it allows compiling out some drivers without > having to mess with the Makefiles. You just don't install all of them. > > Related to that, Peter also mentioned that you (the user, not developer > or packager) could simply disable a single driver, for example as a > temporary hotfix in the case of security problems in a block driver. > That would actually be an argument for making _all_ drivers modules. I am for making all block drivers built as modules. Stefan
Am 10.03.2015 um 14:24 hat Markus Armbruster geschrieben: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: > >> On Tue, 03/10 09:50, Kevin Wolf wrote: > >> > Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > >> > > block/dmg can use additional library (libbz2) to read > >> > > bzip2-compressed files. Make the block driver to be > >> > > a module if libbz2 support is requested, to avoid extra > >> > > library dependency by default. > >> > > > >> > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > >> > > >> > First of all: I don't think this is suitable for trivial. The actual > >> > code change might be small, but the change in behaviour is important and > >> > needs discussion. > >> > > >> > > This might be questionable, to make the thing to be either > >> > > module or built-in depending on build environment, so a > >> > > better idea may be to make it modular unconditionally. > >> > > This block device format isn't used often. > >> > > >> > Yes, I'm concerned that making it conditional might be a bit surprising. > >> > I'd like to hear some more opinions before applying this. > >> > >> I don't see the advantage over making it an unconditional module - condition > >> only makes it a bit more complicated. > >> > >> > > >> > Also, should we consider making some more rarely used image formats > >> > modules even if they don't pull in external dependencies? > >> > >> Sounds reasonable to me. Is the intention to reduce binary size? > > Fair argument, but not a paricularly weighty one. The unused code never > gets paged in, mostly, and isn't particularly large to begin with. > > > Yes, that and also that it allows compiling out some drivers without > > having to mess with the Makefiles. You just don't install all of them. > > Second best solution of the configuration management problem "select a > subset of the available block drivers", good enough when compiling the > unused ones isn't bothersome. But the best solution is still > configuration management capable of disabling optional components. Yes. Though as far as I know, nobody is currently working on that. > > Related to that, Peter also mentioned that you (the user, not developer > > or packager) could simply disable a single driver, for example as a > > temporary hotfix in the case of security problems in a block driver. > > That would actually be an argument for making _all_ drivers modules. > > And pretty much every other optional component. > > For me, avoiding bothersome dependencies is a strong practical argument > for making a something a loadable module. Other benefits of loadable > modules presented so far seem pretty negligible to me. If you want > them, no objection from me, as long as the cost is similarly negligible, > additional complexity for developers, packagers and users in particular. I don't really have a strong opinion either way. I agree that the benefits wouldn't be huge, but then, it would be easy to do and I haven't heard yet of any drawbacks either. There may well be some that I'm not aware of, that's why I'm asking. Kevin
10.03.2015 16:58, Stefan Hajnoczi пишет: > On Tue, Mar 10, 2015 at 10:09 AM, Kevin Wolf <kwolf@redhat.com> wrote: >> Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: >>> On Tue, 03/10 09:50, Kevin Wolf wrote: >>>> Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: >>>> Also, should we consider making some more rarely used image formats >>>> modules even if they don't pull in external dependencies? >>> >>> Sounds reasonable to me. Is the intention to reduce binary size? >> >> Yes, that and also that it allows compiling out some drivers without >> having to mess with the Makefiles. You just don't install all of them. >> >> Related to that, Peter also mentioned that you (the user, not developer >> or packager) could simply disable a single driver, for example as a >> temporary hotfix in the case of security problems in a block driver. >> That would actually be an argument for making _all_ drivers modules. > > I am for making all block drivers built as modules. That might be useful if module loading will be modified a bit, like by loadin modules on demand only. Something like this, search a "foo" block driver in the registered list, found -> use it, if not, try to open block-foo.so (maybe after looking in the "available" internal list before that) and look up in the registered list again. That will make it more useful. Thanks, /mjt
Am 10.03.2015 um 15:01 hat Michael Tokarev geschrieben: > 10.03.2015 16:58, Stefan Hajnoczi пишет: > > On Tue, Mar 10, 2015 at 10:09 AM, Kevin Wolf <kwolf@redhat.com> wrote: > >> Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: > >>> On Tue, 03/10 09:50, Kevin Wolf wrote: > >>>> Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > >>>> Also, should we consider making some more rarely used image formats > >>>> modules even if they don't pull in external dependencies? > >>> > >>> Sounds reasonable to me. Is the intention to reduce binary size? > >> > >> Yes, that and also that it allows compiling out some drivers without > >> having to mess with the Makefiles. You just don't install all of them. > >> > >> Related to that, Peter also mentioned that you (the user, not developer > >> or packager) could simply disable a single driver, for example as a > >> temporary hotfix in the case of security problems in a block driver. > >> That would actually be an argument for making _all_ drivers modules. > > > > I am for making all block drivers built as modules. > > That might be useful if module loading will be modified a bit, > like by loadin modules on demand only. Something like this, > search a "foo" block driver in the registered list, found -> > use it, if not, try to open block-foo.so (maybe after looking > in the "available" internal list before that) and look up in > the registered list again. > > That will make it more useful. The problem with that would be that format probing wouldn't work any more for drivers that aren't loaded yet. Kevin
10.03.2015 17:07, Kevin Wolf wrote: > Am 10.03.2015 um 15:01 hat Michael Tokarev geschrieben: [] >> That might be useful if module loading will be modified a bit, >> like by loadin modules on demand only. Something like this, >> search a "foo" block driver in the registered list, found -> >> use it, if not, try to open block-foo.so (maybe after looking >> in the "available" internal list before that) and look up in >> the registered list again. >> >> That will make it more useful. > > The problem with that would be that format probing wouldn't work any > more for drivers that aren't loaded yet. Probing needed -> run the current "load everything" sequence (maybe limiting stuff being loaded to "block" system when other systems will be there) before actual probing. That should fix both worlds. Alternatively, and this might be even better alternative actually, the more I think about it, is to have a single probe function which recognizes all known formats, no matter if the particular driver is being built or not. This routine will be significantly simpler thant the combined "probe" functionality, it might know much more formats even if a given format is not supported, etc, So that format probing will be its own code which will trigger (attempt) to load a detected driver. Thanks, /mjt
On Tue, Mar 10, 2015 at 05:01:38PM +0300, Michael Tokarev wrote: > 10.03.2015 16:58, Stefan Hajnoczi пишет: > > On Tue, Mar 10, 2015 at 10:09 AM, Kevin Wolf <kwolf@redhat.com> wrote: > >> Am 10.03.2015 um 10:17 hat Fam Zheng geschrieben: > >>> On Tue, 03/10 09:50, Kevin Wolf wrote: > >>>> Am 10.03.2015 um 08:06 hat Michael Tokarev geschrieben: > >>>> Also, should we consider making some more rarely used image formats > >>>> modules even if they don't pull in external dependencies? > >>> > >>> Sounds reasonable to me. Is the intention to reduce binary size? > >> > >> Yes, that and also that it allows compiling out some drivers without > >> having to mess with the Makefiles. You just don't install all of them. > >> > >> Related to that, Peter also mentioned that you (the user, not developer > >> or packager) could simply disable a single driver, for example as a > >> temporary hotfix in the case of security problems in a block driver. > >> That would actually be an argument for making _all_ drivers modules. > > > > I am for making all block drivers built as modules. > > That might be useful if module loading will be modified a bit, > like by loadin modules on demand only. Something like this, > search a "foo" block driver in the registered list, found -> > use it, if not, try to open block-foo.so (maybe after looking > in the "available" internal list before that) and look up in > the registered list again. > > That will make it more useful. Even if all modules are automatically loaded, it gives packagers the ability to ship individual packages for Ceph, GlusterFS, etc to reduce library dependencies. Dynamic loading has advantages but there is no reason to wait for it. Stefan
On Tue, Mar 10, 2015 at 05:38:32PM +0300, Michael Tokarev wrote: > 10.03.2015 17:07, Kevin Wolf wrote: > > Am 10.03.2015 um 15:01 hat Michael Tokarev geschrieben: > [] > >> That might be useful if module loading will be modified a bit, > >> like by loadin modules on demand only. Something like this, > >> search a "foo" block driver in the registered list, found -> > >> use it, if not, try to open block-foo.so (maybe after looking > >> in the "available" internal list before that) and look up in > >> the registered list again. > >> > >> That will make it more useful. > > > > The problem with that would be that format probing wouldn't work any > > more for drivers that aren't loaded yet. > > Probing needed -> run the current "load everything" sequence (maybe > limiting stuff being loaded to "block" system when other systems > will be there) before actual probing. That should fix both worlds. > > Alternatively, and this might be even better alternative actually, > the more I think about it, is to have a single probe function > which recognizes all known formats, no matter if the particular > driver is being built or not. This routine will be significantly > simpler thant the combined "probe" functionality, it might know > much more formats even if a given format is not supported, etc, > So that format probing will be its own code which will trigger > (attempt) to load a detected driver. Or something like /usr/share/magic or /lib/modules/$(uname -r)/modules.alias could be used for probing. For example: be32 at 0 == "QFI\xfb" load qcow2 le32 at 0 == "QED\x00" load qed contains "version=2\n" load vmdk contains "version=1\n" load vmdk QEMU's probe engine matches the rules and loads appropriate modules. Your suggestions are better though because they are simpler and less hassle. In any case, we can solve the probing problem. Stefan
On Tue, Mar 10, 2015 at 10:06:04AM +0300, Michael Tokarev wrote: > block/dmg can use additional library (libbz2) to read > bzip2-compressed files. Make the block driver to be > a module if libbz2 support is requested, to avoid extra > library dependency by default. > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > -- > > This might be questionable, to make the thing to be either > module or built-in depending on build environment, so a > better idea may be to make it modular unconditionally. > This block device format isn't used often. > > block/Makefile.objs | 3 ++- > configure | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) Can you send a series to make all block drivers modules for QEMU 2.4? A separate/later patch series can demand-load modules (and force all for probing), but there is already a packaging advantage if block drivers are built as modules. Stefan
diff --git a/block/Makefile.objs b/block/Makefile.objs index db2933e..440c51f 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -1,7 +1,8 @@ -block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o +block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o +block-obj-$(CONFIG_DMG) += dmg.o block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o block-obj-$(CONFIG_QUORUM) += quorum.o block-obj-y += parallels.o blkdebug.o blkverify.o diff --git a/configure b/configure index 7ba4bcb..1dd5721 100755 --- a/configure +++ b/configure @@ -4772,6 +4772,9 @@ fi if test "$bzip2" = "yes" ; then echo "CONFIG_BZIP2=y" >> $config_host_mak echo "BZIP2_LIBS=-lbz2" >> $config_host_mak + echo "CONFIG_DMG=m" >> $config_host_mak +else + echo "CONFIG_DMG=y" >> $config_host_mak fi if test "$libiscsi" = "yes" ; then
block/dmg can use additional library (libbz2) to read bzip2-compressed files. Make the block driver to be a module if libbz2 support is requested, to avoid extra library dependency by default. Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> -- This might be questionable, to make the thing to be either module or built-in depending on build environment, so a better idea may be to make it modular unconditionally. This block device format isn't used often. block/Makefile.objs | 3 ++- configure | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-)