diff mbox

Questions about gcc linker errors in crypto sub-directory

Message ID 33183CC9F5247A488A2544077AF19020B03DB501@SZXEMA503-MBS.china.huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) Sept. 28, 2016, 1:42 a.m. UTC 
> -----Original Message-----

> From: Daniel P. Berrange [mailto:berrange@redhat.com]

> Sent: Tuesday, September 27, 2016 8:52 PM

> To: Gonglei (Arei)

> Cc: qemu-devel@nongnu.org

> Subject: Re: Questions about gcc linker errors in crypto sub-directory

> 

> On Mon, Sep 26, 2016 at 09:19:36AM +0000, Gonglei (Arei) wrote:

> > > -----Original Message-----

> > > From: Daniel P. Berrange [mailto:berrange@redhat.com]

> > > Sent: Monday, September 26, 2016 5:14 PM

> > > To: Gonglei (Arei)

> > > Cc: qemu-devel@nongnu.org

> > > Subject: Re: Questions about gcc linker errors in crypto sub-directory

> > >

> > > On Mon, Sep 26, 2016 at 09:03:45AM +0000, Gonglei (Arei) wrote:

> > > > Hi Daniel,

> > > >

> > > > I'm coding cryptodev-vhost-user.c as a new cryptodev backend,

> > > > but the gcc report some linker errors:

> > > >

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_crypto_cleanup':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:87:

> > > undefined reference to `vhost_dev_cleanup'

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_crypto_init':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:126:

> > > undefined reference to `vhost_dev_init'

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_user_opened':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:187:

> > > undefined reference to `qemu_chr_find'

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:194:

> > > undefined reference to `qemu_chr_fe_claim_no_fail'

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_user_event':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:213:

> > > undefined reference to `qemu_chr_set_reconnect_time'

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_user_init':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:257:

> > > undefined reference to `qemu_chr_add_handlers'

> > > > crypto/cryptodev-vhost-user.o: In function

> > > `qcrypto_cryptodev_vhost_user_finalize':

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:353:

> > > undefined reference to `qemu_chr_add_handlers'

> > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:354:

> > > undefined reference to `qemu_chr_fe_release'

> > > > collect2: ld returned 1 exit status

> > > > make: *** [qemu-nbd] Error 1

> > > >

> > > > Currently I only change the crypto/Makefile.objs:

> > > >

> > > > diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs

> > > > index b9ad26a..575f64e 100644

> > > > --- a/crypto/Makefile.objs

> > > > +++ b/crypto/Makefile.objs

> > > > @@ -28,6 +28,7 @@ crypto-obj-y += block-qcow.o

> > > >  crypto-obj-y += block-luks.o

> > > >  crypto-obj-y += cryptodev.o

> > > >  crypto-obj-y += cryptodev-builtin.o

> > > > +crypto-obj-y += cryptodev-vhost-user.o

> > >

> > > The $(crypto-obj-y) variable is intended to only contain general purpose

> > > crypto code, since it needs to be linked to all QEMU programs. Your

> > > cryptodev file is specific to system emulators, so must only be linked

> > > to the system emulator targets. Thus, it should not be added to the

> > > crypto-obj-y variable.

> > >

> > > It probably needs to be added to either common-obj-y or obj-y - I can't

> > > remember which is "best"

> > >

> > common-obj-y is fine! So all cryptodev stuff should be use

> > $( crypto-obj-y) variable, right? I'll change them in the next version.

> 

> Looking at this again, it actually makes me think that your

> cryptodev stuff would be better in the backends/ directory.

> 

> That would mean the crypto/ directory is kept as the place for

> generic crypto infrastructure, and avoiding system emulator

> specific device backend code. The backends/Makefile.objs is

> already setup to use common-obj-y and builds various other

> device backend models.

> 

Sounds reasonable and make senses. At present situation, I need
to do below changes for crypto subdirectory:

I'll move them to backends/ directory.  But I still hope you can help to
review cryptdoev stuff in the following virtio-crypto patch serial versions, 
thanks in advance :)

Regards,
-Gonglei

Comments

Daniel P. Berrangé Sept. 28, 2016, 9:09 a.m. UTC | #1 
On Wed, Sep 28, 2016 at 01:42:33AM +0000, Gonglei (Arei) wrote:
> > -----Original Message-----
> > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > Sent: Tuesday, September 27, 2016 8:52 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org
> > Subject: Re: Questions about gcc linker errors in crypto sub-directory
> > 
> > On Mon, Sep 26, 2016 at 09:19:36AM +0000, Gonglei (Arei) wrote:
> > > > -----Original Message-----
> > > > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > > > Sent: Monday, September 26, 2016 5:14 PM
> > > > To: Gonglei (Arei)
> > > > Cc: qemu-devel@nongnu.org
> > > > Subject: Re: Questions about gcc linker errors in crypto sub-directory
> > > >
> > > > On Mon, Sep 26, 2016 at 09:03:45AM +0000, Gonglei (Arei) wrote:
> > > > > Hi Daniel,
> > > > >
> > > > > I'm coding cryptodev-vhost-user.c as a new cryptodev backend,
> > > > > but the gcc report some linker errors:
> > > > >
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_crypto_cleanup':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:87:
> > > > undefined reference to `vhost_dev_cleanup'
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_crypto_init':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:126:
> > > > undefined reference to `vhost_dev_init'
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_user_opened':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:187:
> > > > undefined reference to `qemu_chr_find'
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:194:
> > > > undefined reference to `qemu_chr_fe_claim_no_fail'
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_user_event':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:213:
> > > > undefined reference to `qemu_chr_set_reconnect_time'
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_user_init':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:257:
> > > > undefined reference to `qemu_chr_add_handlers'
> > > > > crypto/cryptodev-vhost-user.o: In function
> > > > `qcrypto_cryptodev_vhost_user_finalize':
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:353:
> > > > undefined reference to `qemu_chr_add_handlers'
> > > > > /mnt/sdb/gonglei/qemu.git/qemu/crypto/cryptodev-vhost-user.c:354:
> > > > undefined reference to `qemu_chr_fe_release'
> > > > > collect2: ld returned 1 exit status
> > > > > make: *** [qemu-nbd] Error 1
> > > > >
> > > > > Currently I only change the crypto/Makefile.objs:
> > > > >
> > > > > diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
> > > > > index b9ad26a..575f64e 100644
> > > > > --- a/crypto/Makefile.objs
> > > > > +++ b/crypto/Makefile.objs
> > > > > @@ -28,6 +28,7 @@ crypto-obj-y += block-qcow.o
> > > > >  crypto-obj-y += block-luks.o
> > > > >  crypto-obj-y += cryptodev.o
> > > > >  crypto-obj-y += cryptodev-builtin.o
> > > > > +crypto-obj-y += cryptodev-vhost-user.o
> > > >
> > > > The $(crypto-obj-y) variable is intended to only contain general purpose
> > > > crypto code, since it needs to be linked to all QEMU programs. Your
> > > > cryptodev file is specific to system emulators, so must only be linked
> > > > to the system emulator targets. Thus, it should not be added to the
> > > > crypto-obj-y variable.
> > > >
> > > > It probably needs to be added to either common-obj-y or obj-y - I can't
> > > > remember which is "best"
> > > >
> > > common-obj-y is fine! So all cryptodev stuff should be use
> > > $( crypto-obj-y) variable, right? I'll change them in the next version.
> > 
> > Looking at this again, it actually makes me think that your
> > cryptodev stuff would be better in the backends/ directory.
> > 
> > That would mean the crypto/ directory is kept as the place for
> > generic crypto infrastructure, and avoiding system emulator
> > specific device backend code. The backends/Makefile.objs is
> > already setup to use common-obj-y and builds various other
> > device backend models.
> > 
> Sounds reasonable and make senses. At present situation, I need
> to do below changes for crypto subdirectory:
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 6d5ddcf..f1a8f13 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -78,6 +78,7 @@ common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
>  
>  common-obj-$(CONFIG_FDT) += device_tree.o
>  
> +common-obj-y += crypto/
>  ######################################################################
>  # qapi
>  
> diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
> index a36d2d9..2632cbc 100644
> --- a/crypto/Makefile.objs
> +++ b/crypto/Makefile.objs
> @@ -27,6 +27,10 @@ crypto-obj-y += block.o
>  crypto-obj-y += block-qcow.o
>  crypto-obj-y += block-luks.o
>  
> +# cryptodev file is specific to system emulators, so must only be linked
> +# to the system emulator targets. 
> +common-obj-y += cryptodev.o
> +
>  # Let the userspace emulators avoid linking gnutls/etc
>  crypto-aes-obj-y = aes.o
> 
> I'll move them to backends/ directory.  But I still hope you can help to
> review cryptdoev stuff in the following virtio-crypto patch serial versions, 
> thanks in advance :)

Sure, i'll review any crypto related stuff.


Regards,
Daniel
diff mbox

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 6d5ddcf..f1a8f13 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -78,6 +78,7 @@  common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
 
 common-obj-$(CONFIG_FDT) += device_tree.o
 
+common-obj-y += crypto/
 ######################################################################
 # qapi
 
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index a36d2d9..2632cbc 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -27,6 +27,10 @@  crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 
+# cryptodev file is specific to system emulators, so must only be linked
+# to the system emulator targets. 
+common-obj-y += cryptodev.o
+
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o